Skip to content

Where people and agents ship software together. The open-source git platform for the whole job: issues, agents, checks and deploys to the edge.

Commit

Fix single sign-on and SCIM findings from a second review

- SAML sign-ins are bound to the browser that set off: a SameSite=None cookie (/auth/sso, HttpOnly) carries a value whose hash the state row keeps, and the provider's cross-site POST must bring it back. Before, someone could send their own request to another person and link that person's provider identity to their own account. - Providers are pointed at the workspace's id, not its name: the entity id, ACS, callback, metadata and SCIM URLs use wsp_..., and every SSO and SCIM path also resolves names and pre-rename names. A rename no longer breaks sign-in or silently stops deprovisioning. - Links keep every stable id the provider gave (sub, Entra oid) and a vouched-for address; SCIM finds people by them, and a person whose subject changed (a new Entra app registration) but whose oid did not is still them. preferred_username is no longer trusted; a string "true" email_verified is. - Erasing an account drops its SSO links and trips and unlinks its SCIM assignments, so a new account can link again. - With SCIM, membership and the SSO session are written only while the assignment is still active (a deprovision mid-sign-in wins); PATCH/PUT write only over the version they read; pushed Groups are accepted and let go so Entra's provisioning never quarantines; SCIM lists filter by userName/externalId/id in SQL; deprovisioning revokes invitations sent to the person's addresses too. - SAML: transient NameIDs are refused with how to fix it; times in any UTC spelling are read. JWT: RSA keys under 2048 bits are refused; a token naming a kid finds a lone unnamed key. - configure_sso keeps saved values for fields left out; the settings page says when join domains cannot be met; save audits say what changed. - Indexes for a person's links and SCIM's externalId (migration 0053). Verified end to end against a local identity worker and D1 (56 checks). Refs #56

G1t-Session:
ses_01m54fhewne5tbgqqh4n8f5fvz
syntaqxcommitted Parent129db18Browse files
21 files+399−1215 files not counted0/21 viewed
+11−11
1520915209 }
1521015210 ],
1521115211 "service_provider": {
15212− "redirect_uri": "https://g1t.sh/auth/sso/flagon-io/callback",
15213− "entity_id": "https://g1t.sh/auth/sso/flagon-io/metadata",
15214− "acs_url": "https://g1t.sh/auth/sso/flagon-io/saml",
15215− "metadata_url": "https://g1t.sh/auth/sso/flagon-io/metadata",
15212+ "redirect_uri": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/callback",
15213+ "entity_id": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/metadata",
15214+ "acs_url": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/saml",
15215+ "metadata_url": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/metadata",
1521615216 "sign_in_url": "https://g1t.sh/auth/sso/flagon-io",
15217− "scim_url": "https://api.g1t.sh/scim/v2/flagon-io"
15217+ "scim_url": "https://api.g1t.sh/scim/v2/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0"
1521815218 }
1521915219 }
1522015220 },
1526315263 }
1526415264 ],
1526515265 "service_provider": {
15266− "redirect_uri": "https://g1t.sh/auth/sso/flagon-io/callback",
15267− "entity_id": "https://g1t.sh/auth/sso/flagon-io/metadata",
15268− "acs_url": "https://g1t.sh/auth/sso/flagon-io/saml",
15269− "metadata_url": "https://g1t.sh/auth/sso/flagon-io/metadata",
15266+ "redirect_uri": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/callback",
15267+ "entity_id": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/metadata",
15268+ "acs_url": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/saml",
15269+ "metadata_url": "https://g1t.sh/auth/sso/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0/metadata",
1527015270 "sign_in_url": "https://g1t.sh/auth/sso/flagon-io",
15271− "scim_url": "https://api.g1t.sh/scim/v2/flagon-io"
15271+ "scim_url": "https://api.g1t.sh/scim/v2/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0"
1527215272 }
1527315273 }
1527415274 },
1531915319 "response": {
1532015320 "token": "g1t_scim_4f1c…",
1532115321 "hint": "9a2e",
15322− "scim_url": "https://api.g1t.sh/scim/v2/flagon-io"
15322+ "scim_url": "https://api.g1t.sh/scim/v2/wsp_01m3xbtsa9d3b4c5e6f7g8h9j0"
1532315323 }
1532415324 },
1532515325 "disable_scim": {
+3−1
1818 None => (rest, "/".to_owned()),
1919 };
2020 let workspace = workspace.trim().to_lowercase();
21− let valid = !workspace.is_empty() && workspace.len() <= 39 && workspace.chars().all(|c| c.is_ascii_alphanumeric() || c == '-');
21+ // The workspace's id (`wsp_…`), or for an older setup its name.
22+ let valid = !workspace.is_empty() && workspace.len() <= 64 && workspace.chars().all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_');
2223 valid.then_some((workspace, path))
2324 }
2425
6667 assert_eq!(split("acme"), Some(("acme".into(), "/".into())));
6768 assert_eq!(split("/Users"), None);
6869 assert_eq!(split("ac me/Users"), None);
70+ assert_eq!(split("wsp_01m54abc/Users"), Some(("wsp_01m54abc".into(), "/Users".into())));
6971 }
7072 }
+81−14
55 //! owners only, as people, and setting it up needs a paid plan. Signing in
66 //! through it is a browser's trip to the provider, so it is not here.
77
8−use g1t_contracts::sso::{ScimToken, SsoConfig, SsoProtocol, SsoSettings, sso_addresses};
8+use g1t_contracts::sso::{ScimToken, SsoConfig, SsoConnection, SsoProtocol, SsoSettings, sso_addresses};
99 use g1t_contracts::{FailureCode, Outcome, Viewer};
1010 use serde_json::{Value, json};
1111 use worker::Result;
8383 pub fn description(self) -> &'static str {
8484 match self {
8585 SsoOp::GetSso => "A workspace's single sign-on through its own identity provider: connection (null when it has none: its id, protocol oidc, oauth2 or saml, config with the protocol's fields, has_client_secret, certificates by SHA-256 fingerprint for SAML, allow_join, tested_at and tested_by, when the first person signed in through it), required (whether members must sign in through it), you_linked (whether you have), identities (the people linked: username, subject and email at the provider, linked_at, last_used_at), and service_provider: the addresses to give the provider (redirect_uri for OpenID Connect and OAuth 2.0; entity_id, acs_url and metadata_url for SAML; sign_in_url, where people start). The client secret is never returned. Owners only, as people.",
86− SsoOp::ConfigureSso => "Set up the workspace's single sign-on, or change it, with every field of the protocol. oidc: issuer (read for its discovery document first), client_id, client_secret, scopes (openid email profile by default). oauth2: authorization_url, token_url, userinfo_url, client_id, client_secret, scopes, subject_claim (sub, then id, by default) and email_claim (email). saml: idp_entity_id, idp_sso_url and certificate (the provider's signing certificates, PEM; several while rotating). client_secret and certificate left out keep what is saved. allow_join (false by default) lets anyone the provider signs in join as a member; join_domains limits that to addresses the provider vouches for at those domains. Moving to another provider (protocol, issuer or entity id) unlinks everyone and stops requiring it. Needs a paid plan. Returns what get_sso does. Recorded as sso.created or sso.updated. Owners only, as people.",
86+ SsoOp::ConfigureSso => "Set up the workspace's single sign-on, or change it; a field left out keeps what is saved for the same protocol. oidc: issuer (read for its discovery document first), client_id, client_secret, scopes (openid email profile by default). oauth2: authorization_url, token_url, userinfo_url, client_id, client_secret, scopes, subject_claim (sub, then id, by default) and email_claim (email). saml: idp_entity_id, idp_sso_url and certificate (the provider's signing certificates, PEM; several while rotating). client_secret and certificate left out keep what is saved. allow_join (false by default) lets anyone the provider signs in join as a member; join_domains limits that to addresses the provider vouches for at those domains. Moving to another provider (protocol, issuer or entity id) unlinks everyone and stops requiring it. Needs a paid plan. Returns what get_sso does. Recorded as sso.created or sso.updated. Owners only, as people.",
8787 SsoOp::SetSsoRequired => "Whether members must sign in through the provider: then a member uses the workspace only from a session that did in the last day, and with tokens and keys once linked; held out otherwise, keeping their place. Owners are never held, and outside collaborators are not asked. Requiring it needs a paid plan and you to have signed in through it yourself (you_linked); stopping never does. Recorded as sso.required or sso.not_required. Owners only, as people.",
8888 SsoOp::DeleteSso => "Remove the workspace's single sign-on: forgets the provider and who everyone is there, and stops requiring it. Members keep their place. Never needs a plan. Recorded as sso.deleted. Owners only, as people.",
8989 SsoOp::ListSsoIdentities => "The people linked to the workspace's provider: username, subject (their id at the provider), email (as the provider gave it, for showing), linked_at and last_used_at. Owners only, as people.",
182182
183183 /// The settings, with where to point the provider.
184184 fn with_addresses(settings: SsoSettings, site: &str, api: &str) -> Value {
185− let addresses = sso_addresses(site, &settings.slug);
185+ let addresses = sso_addresses(site, &settings.workspace_id, &settings.slug);
186186 let mut out = serde_json::to_value(&settings).unwrap_or_default();
187187 out["service_provider"] = serde_json::to_value(addresses).unwrap_or_default();
188− out["service_provider"]["scim_url"] = json!(scim_url(api, &settings.slug));
188+ out["service_provider"]["scim_url"] = json!(scim_url(api, &settings.workspace_id));
189189 out
190190 }
191191
192−/// The SCIM 2.0 base URL for a workspace (scim.rs).
193−fn scim_url(api: &str, slug: &str) -> String {
194− format!("{}/scim/v2/{}", api.trim_end_matches('/'), slug.to_lowercase())
192+/// The SCIM 2.0 base URL for a workspace, by its id, which a rename never
193+/// changes (scim.rs).
194+fn scim_url(api: &str, workspace_id: &str) -> String {
195+ format!("{}/scim/v2/{}", api.trim_end_matches('/'), workspace_id)
196+}
197+
198+/// What `configure_sso` was sent, over what is saved for the same protocol:
199+/// a field left out keeps its value, so changing one thing (a rotated
200+/// secret) changes nothing else.
201+fn merged(input: &Value, saved: Option<&SsoConnection>, protocol: SsoProtocol) -> (SsoConfig, bool) {
202+ let mut config = config_of(input);
203+ let same = saved.filter(|saved| saved.protocol == protocol);
204+ if let Some(saved) = same {
205+ let kept = &saved.config;
206+ let keep = |sent: &mut Option<String>, kept: &Option<String>, field: &str| {
207+ if input.get(field).is_none() {
208+ *sent = kept.clone();
209+ }
210+ };
211+ keep(&mut config.issuer, &kept.issuer, "issuer");
212+ keep(&mut config.client_id, &kept.client_id, "client_id");
213+ keep(&mut config.scopes, &kept.scopes, "scopes");
214+ keep(&mut config.authorization_url, &kept.authorization_url, "authorization_url");
215+ keep(&mut config.token_url, &kept.token_url, "token_url");
216+ keep(&mut config.userinfo_url, &kept.userinfo_url, "userinfo_url");
217+ keep(&mut config.subject_claim, &kept.subject_claim, "subject_claim");
218+ keep(&mut config.email_claim, &kept.email_claim, "email_claim");
219+ keep(&mut config.idp_entity_id, &kept.idp_entity_id, "idp_entity_id");
220+ keep(&mut config.idp_sso_url, &kept.idp_sso_url, "idp_sso_url");
221+ if input.get("join_domains").is_none() {
222+ config.join_domains = kept.join_domains.clone();
223+ }
224+ }
225+ let allow_join = flag(input, "allow_join").unwrap_or_else(|| same.is_some_and(|saved| saved.allow_join));
226+ (config, allow_join)
195227 }
196228
197229 pub async fn run(op: SsoOp, services: &Services, viewer: &Viewer, input: &Value) -> Result<Outcome<Value>> {
225257 if input.get("allow_join").is_some_and(|v| !v.is_null()) && flag(input, "allow_join").is_none() {
226258 return Ok(Outcome::fail(FailureCode::Invalid, "allow_join is true or false."));
227259 }
260+ let before: Outcome<SsoSettings> = g1t_kit::call(identity, "sso_settings", &json!({ "actor": actor, "slug": workspace })).await?;
261+ let before = match before {
262+ Outcome::Ok(before) => before,
263+ Outcome::Fail(failure) => return Ok(Outcome::Fail(failure)),
264+ };
265+ let (config, allow_join) = merged(input, before.connection.as_ref(), protocol);
228266 let saved: Outcome<SsoSettings> = g1t_kit::call(
229267 identity,
230268 "save_sso",
232270 "actor": actor,
233271 "slug": workspace,
234272 "protocol": protocol,
235− "config": config_of(input),
273+ "config": config,
236274 "client_secret": text(input, "client_secret"),
237275 "certificate": text(input, "certificate"),
238− "allow_join": flag(input, "allow_join").unwrap_or(false),
276+ "allow_join": allow_join,
239277 "surface": surface,
240278 }),
241279 )
269307 let made: Outcome<ScimToken> =
270308 g1t_kit::call(identity, "enable_scim", &json!({ "actor": actor, "slug": workspace, "surface": surface })).await?;
271309 match made {
272− Outcome::Ok(made) => Outcome::Ok(json!({ "token": made.token, "hint": made.hint, "scim_url": scim_url(api, &workspace) })),
310+ Outcome::Ok(made) => Outcome::Ok(json!({ "token": made.token, "hint": made.hint, "scim_url": scim_url(api, &made.workspace_id) })),
273311 Outcome::Fail(failure) => Outcome::Fail(failure),
274312 }
275313 }
325363 }
326364
327365 #[test]
366+ fn a_field_left_out_keeps_what_is_saved() {
367+ let saved = SsoConnection {
368+ id: "sso_1".into(),
369+ protocol: SsoProtocol::Oidc,
370+ config: SsoConfig { issuer: Some("https://acme.okta.com".into()), client_id: Some("c".into()), join_domains: vec!["acme.com".into()], ..SsoConfig::default() },
371+ has_client_secret: true,
372+ certificates: vec![],
373+ allow_join: true,
374+ tested_at: None,
375+ tested_by: None,
376+ created_at: String::new(),
377+ updated_at: String::new(),
378+ };
379+ // Rotating the secret alone changes nothing else.
380+ let (config, allow_join) = merged(&json!({ "protocol": "oidc", "client_secret": "new" }), Some(&saved), SsoProtocol::Oidc);
381+ assert_eq!(config.issuer.as_deref(), Some("https://acme.okta.com"));
382+ assert_eq!(config.join_domains, vec!["acme.com".to_owned()]);
383+ assert!(allow_join);
384+ // Sent empty is changed; another protocol starts afresh.
385+ let (config, allow_join) = merged(&json!({ "protocol": "oidc", "join_domains": [], "allow_join": false }), Some(&saved), SsoProtocol::Oidc);
386+ assert!(config.join_domains.is_empty());
387+ assert!(!allow_join);
388+ let (config, allow_join) = merged(&json!({ "protocol": "saml" }), Some(&saved), SsoProtocol::Saml);
389+ assert_eq!(config.issuer, None);
390+ assert!(!allow_join);
391+ }
392+
393+ #[test]
328394 fn the_answer_says_where_to_point_the_provider() {
329− let settings = SsoSettings { slug: "acme".into(), connection: None, required: false, you_linked: false, identities: vec![], scim: None };
395+ let settings = SsoSettings { slug: "acme".into(), workspace_id: "wsp_1".into(), connection: None, required: false, you_linked: false, identities: vec![], scim: None };
330396 let out = with_addresses(settings, "https://g1t.sh", "https://api.g1t.sh");
331− assert_eq!(out["service_provider"]["scim_url"], "https://api.g1t.sh/scim/v2/acme");
332− assert_eq!(out["service_provider"]["acs_url"], "https://g1t.sh/auth/sso/acme/saml");
333− assert_eq!(out["service_provider"]["redirect_uri"], "https://g1t.sh/auth/sso/acme/callback");
397+ assert_eq!(out["service_provider"]["scim_url"], "https://api.g1t.sh/scim/v2/wsp_1");
398+ assert_eq!(out["service_provider"]["acs_url"], "https://g1t.sh/auth/sso/wsp_1/saml");
399+ assert_eq!(out["service_provider"]["redirect_uri"], "https://g1t.sh/auth/sso/wsp_1/callback");
400+ assert_eq!(out["service_provider"]["sign_in_url"], "https://g1t.sh/auth/sso/acme");
334401 assert_eq!(out["connection"], Value::Null);
335402 }
336403 }
+11−6
2828
2929 | | |
3030 | --- | --- |
31− | Redirect URI (OpenID Connect, OAuth 2.0) | `https://g1t.sh/auth/sso/<workspace>/callback` |
32− | Entity id, audience (SAML) | `https://g1t.sh/auth/sso/<workspace>/metadata` |
33− | Assertion consumer service URL, HTTP-POST (SAML) | `https://g1t.sh/auth/sso/<workspace>/saml` |
34− | Metadata (SAML) | `https://g1t.sh/auth/sso/<workspace>/metadata` |
31+ | Redirect URI (OpenID Connect, OAuth 2.0) | `https://g1t.sh/auth/sso/<workspace-id>/callback` |
32+ | Entity id, audience (SAML) | `https://g1t.sh/auth/sso/<workspace-id>/metadata` |
33+ | Assertion consumer service URL, HTTP-POST (SAML) | `https://g1t.sh/auth/sso/<workspace-id>/saml` |
34+ | Metadata (SAML) | `https://g1t.sh/auth/sso/<workspace-id>/metadata` |
3535
36+ They name the workspace by its id (`wsp_…`), which never changes, so
37+ renaming the workspace leaves your provider's setup as it is. The page
38+ shows them with the id filled in.
39+
3640 3. Copy what the provider gives you back into the page:
3741
3842 - **OpenID Connect**: the issuer (where its
190194 stays. **Owners are never removed this way**; the audit log says so, and
191195 you remove one by hand if they should go.
192196 - People, not groups. Assign a group to the app in your provider and its
193− members are provisioned one by one; turn off pushing groups.
197+ members are provisioned one by one. Groups your provider pushes are
198+ accepted and not kept, so pushing them never fails.
194199
195200 To turn it on, open **Single sign-on → SCIM provisioning** and choose
196201 **Turn on SCIM**. Give your provider:
197202
198203 | | |
199204 | --- | --- |
200−| SCIM base URL (tenant URL) | `https://api.g1t.sh/scim/v2/<workspace>` |
205+| SCIM base URL (tenant URL) | `https://api.g1t.sh/scim/v2/<workspace-id>` |
201206 | Authentication | Bearer token: the SCIM token, shown once |
202207 | Unique identifier | `userName` |
203208 | Supported | Create, update (PUT and PATCH), deactivate and delete users; filtering by `userName`, `externalId`, `id` or `emails.value` |
+0−0

Binary or large file; its contents are not shown.

+17−7
33
44 import { ssoAddresses } from "@g1t/contracts/sso";
55
6−import { SSO_ERROR_COOKIE, afterSso, errorCookie, readErrorCookie, samlPath } from "./sso.ts";
6+import { SSO_ERROR_COOKIE, afterSso, bindingCookie, errorCookie, readBinding, readErrorCookie, samlPath } from "./sso.ts";
77
88 test("only a workspace's SAML address is answered before React Router", () => {
99 assert.equal(samlPath("/auth/sso/acme/saml"), "acme");
1010 assert.equal(samlPath("/auth/sso/Acme-Co/saml/"), "acme-co");
11+ assert.equal(samlPath("/auth/sso/wsp_01m54abc/saml"), "wsp_01m54abc");
1112 assert.equal(samlPath("/auth/sso/acme/callback"), null);
1213 assert.equal(samlPath("/auth/sso/acme/saml/extra"), null);
1314 assert.equal(samlPath("/auth/sso/-acme/saml"), null);
3031 assert.equal(readErrorCookie(null), null);
3132 });
3233
33−test("a workspace's provider is pointed at its own addresses", () => {
34− assert.deepEqual(ssoAddresses("https://g1t.sh/", "Acme"), {
35− entityId: "https://g1t.sh/auth/sso/acme/metadata",
36− metadataUrl: "https://g1t.sh/auth/sso/acme/metadata",
37− acsUrl: "https://g1t.sh/auth/sso/acme/saml",
38− callbackUrl: "https://g1t.sh/auth/sso/acme/callback",
34+test("a workspace's provider is pointed at its id, which a rename never changes", () => {
35+ assert.deepEqual(ssoAddresses("https://g1t.sh/", "wsp_1", "Acme"), {
36+ entityId: "https://g1t.sh/auth/sso/wsp_1/metadata",
37+ metadataUrl: "https://g1t.sh/auth/sso/wsp_1/metadata",
38+ acsUrl: "https://g1t.sh/auth/sso/wsp_1/saml",
39+ callbackUrl: "https://g1t.sh/auth/sso/wsp_1/callback",
40+ signInUrl: "https://g1t.sh/auth/sso/acme",
3941 });
4042 });
43+
44+test("the SAML binding cookie crosses sites to /auth/sso alone", () => {
45+ const value = "a".repeat(64);
46+ assert.match(bindingCookie(value), /Path=\/auth\/sso; HttpOnly; Secure; SameSite=None; Max-Age=600/);
47+ assert.equal(readBinding(`x=1; ${bindingCookie(value).split(";")[0]}`), value);
48+ assert.equal(readBinding("g1t_sso_saml=nothex"), null);
49+ assert.match(bindingCookie(""), /Max-Age=0/);
50+});
+22−2
1111 export const SSO_STATE_COOKIE = "g1t_sso_state";
1212 /** Why a SAML sign-in did not work, shown once on the page it lands on. */
1313 export const SSO_ERROR_COOKIE = "g1t_sso_error";
14+/**
15+ * Binds a SAML trip to the browser that set off. SameSite=None, so the
16+ * provider's cross-site POST carries it back (the session cookie, Lax, is
17+ * not sent with it); HttpOnly, and only to /auth/sso.
18+ */
19+export const SAML_BINDING_COOKIE = "g1t_sso_saml";
20+
21+/** A `Set-Cookie` value for the binding, for ten minutes; empty clears it. */
22+export function bindingCookie(value: string): string {
23+ return `${SAML_BINDING_COOKIE}=${value}; Path=/auth/sso; HttpOnly; Secure; SameSite=None; Max-Age=${value ? 600 : 0}`;
24+}
25+
26+/** The binding cookie's value, or null. Hex only. */
27+export function readBinding(header: string | null): string | null {
28+ for (const part of (header ?? "").split(";")) {
29+ const [key, value] = part.trim().split("=");
30+ if (key === SAML_BINDING_COOKIE && value && /^[0-9a-f]{64}$/.test(value)) return value;
31+ }
32+ return null;
33+}
1434
15−/** The workspace a `POST /auth/sso/<slug>/saml` is for, or null. */
35+/** The workspace (its id, or a name) a `POST /auth/sso/<workspace>/saml` is for, or null. */
1636 export function samlPath(pathname: string): string | null {
17− const match = /^\/auth\/sso\/([a-z0-9](?:[a-z0-9-]{0,38}))\/saml\/?$/i.exec(pathname);
37+ const match = /^\/auth\/sso\/([a-z0-9][a-z0-9_-]{0,63})\/saml\/?$/i.exec(pathname);
1838 return match ? match[1].toLowerCase() : null;
1939 }
2040
+7−2
1+import { data } from "react-router";
2+
13 import { ssoAddresses } from "@g1t/contracts";
24
35 import type { Route } from "./+types/auth-sso-metadata";
911 * response. The same for everyone; it says nothing about the workspace.
1012 */
1113 export async function loader({ request, params }: Route.LoaderArgs) {
12− const addresses = ssoAddresses(new URL(request.url).origin, params.owner);
14+ // Keyed by the workspace's id, whatever names it here.
15+ const summary = await sso.summary(params.owner);
16+ if (!summary.workspace_id) throw data("No such workspace.", { status: 404 });
17+ const addresses = ssoAddresses(new URL(request.url).origin, summary.workspace_id, params.owner);
1318 const xml = await sso.metadata(addresses.entityId, addresses.acsUrl);
1419 return new Response(xml, {
1520 headers: {
1621 "content-type": "application/samlmetadata+xml; charset=utf-8",
1722 "cache-control": "public, max-age=3600",
18− "content-disposition": `inline; filename="g1t-${params.owner.toLowerCase()}-metadata.xml"`,
23+ "content-disposition": `inline; filename="g1t-${summary.workspace_id}-metadata.xml"`,
1924 },
2025 });
2126 }
+5−3
88 import { cookie } from "../lib/github";
99 import { page } from "../lib/meta";
1010 import { safeNext } from "../lib/next";
11−import { SSO_STATE_COOKIE } from "../lib/sso";
11+import { SSO_STATE_COOKIE, bindingCookie } from "../lib/sso";
1212 import { sso } from "../lib/sso.server";
1313 import { assertSameOrigin, nextPath, requireUser, sessionTokenOf } from "../lib/session.server";
1414
3131 if (!token) return { slug, error: "Single sign-on is for people signed in on this site, not for access tokens." };
3232 const summary = await sso.summary(slug);
3333 if (!summary.enabled) throw data({ slug, error: `${slug} has not set up single sign-on.` }, { status: 404 });
34− const addresses = ssoAddresses(new URL(request.url).origin, slug);
34+ const addresses = ssoAddresses(new URL(request.url).origin, summary.workspace_id, slug);
3535 const started = await sso.start({
3636 user,
3737 session_token: token,
4141 next,
4242 });
4343 if (!started.ok) return { slug, error: started.error.message };
44− const headers = started.value.state ? { "set-cookie": cookie(SSO_STATE_COOKIE, started.value.state, 600) } : undefined;
44+ const headers = new Headers();
45+ if (started.value.state) headers.append("set-cookie", cookie(SSO_STATE_COOKIE, started.value.state, 600));
46+ if (started.value.binding) headers.append("set-cookie", bindingCookie(started.value.binding));
4547 throw redirect(started.value.url, { headers });
4648 }
4749
+7−3
4949 return {
5050 settings: unwrap(settings),
5151 plan: isFailed(free) ? free : free.includes(slug) ? ("free" as const) : ("paid" as const),
52− addresses: ssoAddresses(new URL(request.url).origin, slug),
52+ addresses: ssoAddresses(new URL(request.url).origin, unwrap(settings).workspace_id, slug),
5353 };
5454 }
5555
176176 /** SCIM: the provider assigns and unassigns people. */
177177 function ScimSection({ settings, slug, disabled, answer }: { settings: SsoSettings; slug: string; disabled: boolean; answer?: Answer }) {
178178 const scim = settings.scim ?? null;
179− const url = scimUrl(useAddresses().api, slug);
179+ const url = scimUrl(useAddresses().api, settings.workspace_id);
180180 return (
181181 <section aria-labelledby="sso-scim">
182182 <h2 id="sso-scim" className="font-medium">
434434 </SwitchCard>
435435 <Field
436436 label="Only addresses at"
437− hint="Who may join, when joining is on: people whose address your provider vouches for is at one of these domains, such as acme.com. Empty: anyone your provider signs in."
437+ hint={
438+ protocol === "oauth2"
439+ ? "Plain OAuth 2.0 never vouches for an address, so with domains set nobody can join; leave it empty, or use OpenID Connect."
440+ : "Who may join, when joining is on: people whose address your provider vouches for is at one of these domains, such as acme.com. Microsoft Entra ID vouches for none over OpenID Connect; use SAML to limit by domain. Empty: anyone your provider signs in."
441+ }
438442 >
439443 <Input name="join_domains" defaultValue={(connection?.config.join_domains ?? []).join(", ")} placeholder="acme.com" />
440444 </Field>
+14−8
11 import { ssoClient } from "@g1t/contracts";
22
3−import { afterSso, errorCookie } from "../app/lib/sso";
3+import { afterSso, bindingCookie, errorCookie, readBinding } from "../app/lib/sso";
44
55 /**
66 * A workspace's identity provider posting its SAML response
1212 * going, or back to the page that says what went wrong.
1313 */
1414 export async function receiveSaml(env: Env, request: Request, slug: string): Promise<Response> {
15− const back = (message: string) =>
16− new Response(null, {
17− status: 303,
18− headers: { location: `/auth/sso/${slug}/saml`, "set-cookie": errorCookie(message), "cache-control": "no-store" },
19− });
15+ // The binding cookie set when this browser set off: identity accepts the
16+ // response only for the trip it binds. Spent either way.
17+ const binding = readBinding(request.headers.get("cookie"));
18+ const back = (message: string) => {
19+ const headers = new Headers({ location: `/auth/sso/${slug}/saml`, "cache-control": "no-store" });
20+ headers.append("set-cookie", errorCookie(message));
21+ headers.append("set-cookie", bindingCookie(""));
22+ return new Response(null, { status: 303, headers });
23+ };
2024 let samlResponse = "";
2125 try {
2226 const form = await request.formData();
2630 }
2731 if (!samlResponse) return back("The identity provider sent no SAML response.");
2832 const finished = await ssoClient(env.IDENTITY)
29− .saml(slug, samlResponse)
33+ .saml(slug, samlResponse, binding)
3034 .catch(() => null);
3135 if (!finished) return back("g1t could not check the identity provider's response just now. Try again in a minute.");
3236 if (!finished.ok) return back(finished.error.message);
33− return new Response(null, { status: 303, headers: { location: afterSso(finished.value), "cache-control": "no-store" } });
37+ const headers = new Headers({ location: afterSso(finished.value), "cache-control": "no-store" });
38+ headers.append("set-cookie", bindingCookie(""));
39+ return new Response(null, { status: 303, headers });
3440 }
+33−13
158158 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
159159 pub struct SsoSettings {
160160 pub slug: String,
161+ /// The workspace's id, which never changes: what the provider is
162+ /// pointed at (`sso_addresses`), so renaming the workspace breaks
163+ /// nothing.
164+ #[serde(default)]
165+ pub workspace_id: String,
161166 pub connection: Option<SsoConnection>,
162167 /// Whether members must sign in through it.
163168 pub required: bool,
201206 pub struct ScimToken {
202207 pub token: String,
203208 pub hint: String,
209+ /// What the SCIM URL names the workspace by.
210+ #[serde(default)]
211+ pub workspace_id: String,
204212 }
205213
206214 /// `scim_request`: one SCIM request, as the API received it at
232240 /// and the notice for someone it holds out.
233241 #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
234242 pub struct SsoSummary {
243+ #[serde(default)]
244+ pub workspace_id: String,
235245 pub enabled: bool,
236246 pub protocol: Option<SsoProtocol>,
237247 pub required: bool,
316326 /// Where to send the browser.
317327 pub url: String,
318328 /// OpenID Connect and OAuth 2.0: the state, for a cookie that binds
319− /// the trip to this browser. Absent for SAML, whose response is posted
320− /// cross-site, without cookies; its request id binds it instead.
329+ /// the trip to this browser. Absent for SAML.
321330 pub state: Option<String>,
331+ /// SAML: a value for a SameSite=None cookie, which the provider's
332+ /// cross-site POST carries back (`SsoSamlArgs::binding`), binding the
333+ /// response to the browser that set off.
334+ #[serde(default)]
335+ pub binding: Option<String>,
322336 }
323337
324338 /// `sso_finish`: the provider sent the browser back with a code.
335349 pub struct SsoSamlArgs {
336350 pub slug: String,
337351 pub saml_response: String,
352+ /// The binding cookie's value (`SsoStart::binding`).
353+ #[serde(default)]
354+ pub binding: Option<String>,
338355 }
339356
340357 #[derive(Debug, Serialize, Deserialize)]
369386 pub sign_in_url: String,
370387 }
371388
372−pub fn sso_addresses(site: &str, slug: &str) -> SsoAddresses {
373− let start = format!("{}/auth/sso/{}", site.trim_end_matches('/'), slug.trim().to_lowercase());
389+/// What the provider is pointed at is keyed by the workspace's id, which a
390+/// rename never changes; where people start is its name.
391+pub fn sso_addresses(site: &str, workspace_id: &str, slug: &str) -> SsoAddresses {
392+ let site = site.trim_end_matches('/');
393+ let at = format!("{site}/auth/sso/{}", workspace_id.trim());
374394 SsoAddresses {
375− redirect_uri: format!("{start}/callback"),
376− entity_id: format!("{start}/metadata"),
377− acs_url: format!("{start}/saml"),
378− metadata_url: format!("{start}/metadata"),
379− sign_in_url: start,
395+ redirect_uri: format!("{at}/callback"),
396+ entity_id: format!("{at}/metadata"),
397+ acs_url: format!("{at}/saml"),
398+ metadata_url: format!("{at}/metadata"),
399+ sign_in_url: format!("{site}/auth/sso/{}", slug.trim().to_lowercase()),
380400 }
381401 }
382402
390410
391411 #[test]
392412 fn addresses_are_the_workspaces_own() {
393− let at = sso_addresses("https://g1t.sh/", " Acme ");
394− assert_eq!(at.redirect_uri, "https://g1t.sh/auth/sso/acme/callback");
395− assert_eq!(at.entity_id, "https://g1t.sh/auth/sso/acme/metadata");
396− assert_eq!(at.acs_url, "https://g1t.sh/auth/sso/acme/saml");
413+ let at = sso_addresses("https://g1t.sh/", "wsp_1", " Acme ");
414+ assert_eq!(at.redirect_uri, "https://g1t.sh/auth/sso/wsp_1/callback");
415+ assert_eq!(at.entity_id, "https://g1t.sh/auth/sso/wsp_1/metadata");
416+ assert_eq!(at.acs_url, "https://g1t.sh/auth/sso/wsp_1/saml");
397417 assert_eq!(at.sign_in_url, "https://g1t.sh/auth/sso/acme");
398418 }
399419 }
+19−8
5454
5555 export type SsoSettings = {
5656 slug: string;
57+ /** The workspace's id, which never changes: what the provider is pointed at. */
58+ workspace_id: string;
5759 connection: SsoConnection | null;
5860 required: boolean;
5961 /** Whether the owner asking has signed in through it, which requiring it needs. */
8385 };
8486
8587 export type SsoSummary = {
88+ workspace_id: string;
8689 enabled: boolean;
8790 protocol: SsoProtocol | null;
8891 required: boolean;
125128 unlink: (actor: User, slug: string, username: string) =>
126129 call<Result<boolean>>("unlink_sso_identity", { actor, slug, username, surface: "web" }),
127130 start: (input: { user: User; session_token: string; slug: string; redirect_uri: string; entity_id?: string | null; next: string }) =>
128− call<Result<{ url: string; state: string | null }>>("sso_start", input),
131+ call<Result<{ url: string; state: string | null; binding: string | null }>>("sso_start", input),
129132 finish: (input: { slug: string; session_token: string; state: string; code: string }) =>
130133 call<Result<SsoFinished>>("sso_finish", input),
131− saml: (slug: string, samlResponse: string) => call<Result<SsoFinished>>("sso_saml", { slug, saml_response: samlResponse }),
134+ saml: (slug: string, samlResponse: string, binding: string | null) =>
135+ call<Result<SsoFinished>>("sso_saml", { slug, saml_response: samlResponse, binding }),
132136 metadata: (entityId: string, acsUrl: string) => call<string>("sso_metadata", { entity_id: entityId, acs_url: acsUrl }),
133137 enableScim: (actor: User, slug: string) => call<Result<{ token: string; hint: string }>>("enable_scim", { actor, slug, surface: "web" }),
134138 disableScim: (actor: User, slug: string) => call<Result<boolean>>("disable_scim", { actor, slug, surface: "web" }),
140144 * entity id (its metadata's address), the assertion consumer service, and
141145 * the callback for OpenID Connect and OAuth 2.0.
142146 */
143−/** The SCIM 2.0 base URL for a workspace, on the API. */
144−export function scimUrl(api: string, slug: string): string {
145− return `${api.replace(/\/+$/, "")}/scim/v2/${encodeURIComponent(slug.toLowerCase())}`;
147+/** The SCIM 2.0 base URL for a workspace, on the API, by its id. */
148+export function scimUrl(api: string, workspaceId: string): string {
149+ return `${api.replace(/\/+$/, "")}/scim/v2/${encodeURIComponent(workspaceId)}`;
146150 }
147151
148−export function ssoAddresses(origin: string, slug: string) {
149− const base = `${origin.replace(/\/+$/, "")}/auth/sso/${encodeURIComponent(slug.toLowerCase())}`;
150− return { entityId: `${base}/metadata`, metadataUrl: `${base}/metadata`, acsUrl: `${base}/saml`, callbackUrl: `${base}/callback` };
152+export function ssoAddresses(origin: string, workspaceId: string, slug: string) {
153+ const site = origin.replace(/\/+$/, "");
154+ const base = `${site}/auth/sso/${encodeURIComponent(workspaceId)}`;
155+ return {
156+ entityId: `${base}/metadata`,
157+ metadataUrl: `${base}/metadata`,
158+ acsUrl: `${base}/saml`,
159+ callbackUrl: `${base}/callback`,
160+ signInUrl: `${site}/auth/sso/${encodeURIComponent(slug.toLowerCase())}`,
161+ };
151162 }
+18−0
1+-- Single sign-on, after review. See src/sso.rs.
2+
3+-- A SAML trip is bound to the browser that set off: the hash of a value
4+-- kept in a SameSite=None cookie, which the provider's cross-site POST
5+-- carries back (the session cookie, SameSite=Lax, does not).
6+ALTER TABLE sso_states ADD COLUMN browser TEXT;
7+
8+-- Every stable id the provider gave for the person (the subject first,
9+-- and Entra's object id), as a JSON array: SCIM finds them by any of
10+-- them, and a new app registration with another subject is still them.
11+ALTER TABLE sso_identities ADD COLUMN ids TEXT NOT NULL DEFAULT '[]';
12+-- Their address, when the provider vouched for it.
13+ALTER TABLE sso_identities ADD COLUMN trusted_email TEXT;
14+
15+-- A person's links, for their tokens and keys on every request.
16+CREATE INDEX IF NOT EXISTS sso_identities_by_user ON sso_identities (user_id);
17+-- SCIM looks people up by externalId.
18+CREATE INDEX IF NOT EXISTS scim_users_by_external ON scim_users (workspace_id, external_id);
+4−0
267267 "two_factor_challenges",
268268 "email_tokens",
269269 "github_states",
270+ "sso_identities",
271+ "sso_states",
270272 "workspace_members",
271273 "team_members",
272274 ] {
275277 sql.extend([
276278 // Deploy keys it added to repositories.
277279 ("DELETE FROM deploy_keys WHERE created_by = ?1".to_owned(), 1),
280+ // Its SCIM assignments link to nobody; the provider still has them.
281+ ("UPDATE scim_users SET user_id = NULL WHERE user_id = ?1".to_owned(), 1),
278282 // g1t keeps no GitHub token for it any more.
279283 ("UPDATE github_accounts SET tokens = NULL WHERE user_id = ?1".to_owned(), 1),
280284 ("DELETE FROM repo_grants WHERE principal_kind = 'user' AND principal_id = ?1".to_owned(), 1),
+147−43
135135 redirect_uri: String,
136136 audience: Option<String>,
137137 next: String,
138+ #[serde(default)]
139+ browser: Option<String>,
138140 }
139141
140142 /// Who the provider says signed in.
165167 }
166168 }
167169
168−/// What an OpenID Connect ID token says: Entra's `oid` and, for its tenant's
169−/// own members (never a guest, `#EXT#`), the user principal name are stable
170−/// ids SCIM may know them by.
170+/// What an OpenID Connect ID token says. For Entra, its `oid` too: the
171+/// person's object id in the tenant, the same in every app registration
172+/// (`sub` is not), and what SCIM may know them by. Never its
173+/// `preferred_username`, which is mutable and, for a guest, their home
174+/// address.
171175 fn said_by_oidc(claims: &Value, issuer: &str) -> Option<Said> {
172176 let subject = oauth::claim(claims, "sub")?;
173− let mut said = Said::new(subject, oauth::claim(claims, "email"), claims["email_verified"].as_bool() == Some(true));
177+ // Some providers send the flag as a string.
178+ let verified = claims["email_verified"].as_bool().or_else(|| claims["email_verified"].as_str().map(|v| v.eq_ignore_ascii_case("true"))) == Some(true);
179+ let mut said = Said::new(subject, oauth::claim(claims, "email"), verified);
174180 if issuer.starts_with("https://login.microsoftonline.com/") {
175− said.ids.extend(oauth::claim(claims, "oid"));
176− said.ids.extend(oauth::claim(claims, "preferred_username").filter(|upn| !upn.contains("#EXT#")).map(|upn| upn.to_lowercase()));
181+ said.ids.extend(oauth::claim(claims, "oid").filter(|oid| !said.ids.contains(oid)));
177182 }
178183 Some(said)
179184 }
180185
186+/// Whether two lists of a person's ids share one.
187+fn same_person(stored: &[String], said: &[String]) -> bool {
188+ stored.iter().any(|id| said.iter().any(|other| other.eq_ignore_ascii_case(id)))
189+}
190+
181191 /// Why `said` may not join by its address, when the connection names the
182192 /// domains that may: an address the provider vouches for, at one of them.
183193 fn join_domain_refusal(domains: &[String], said: &Said, slug: &str) -> Option<String> {
342352 })
343353 }
344354
355+ /// The workspace `key` names, as `(id, slug)`: its id (`wsp_…`, what a
356+ /// provider is pointed at), its name, or a name it had before a rename.
357+ pub(crate) async fn sso_workspace(&self, key: &str) -> Result<Option<(String, String)>> {
358+ #[derive(Deserialize)]
359+ struct Found {
360+ id: String,
361+ slug: String,
362+ }
363+ let key = key.trim();
364+ let found = self
365+ .db
366+ .prepare(
367+ "SELECT id, slug FROM workspaces WHERE deleted_at IS NULL AND (id = ?1 OR slug = lower(?1)
368+ OR id = (SELECT workspace_id FROM workspace_redirects WHERE old_slug = lower(?1)))
369+ ORDER BY id = ?1 DESC, slug = lower(?1) DESC LIMIT 1",
370+ )
371+ .bind(&[key.into()])?
372+ .first::<Found>(None)
373+ .await?;
374+ Ok(found.map(|f| (f.id, f.slug)))
375+ }
376+
345377 async fn require_sso_of(&self, workspace_id: &str) -> Result<bool> {
346378 #[derive(Deserialize)]
347379 struct Row {
378410
379411 async fn settings_of(&self, workspace_id: &str, slug: &str, actor: &User) -> Result<SsoSettings> {
380412 let row = self.connection_where("workspace_id", workspace_id).await?;
381− let (identities, you_linked) = match &row {
413+ let you_linked = match &row {
414+ Some(row) => self.sso_linked(&row.id, &actor.id).await?,
415+ None => false,
416+ };
417+ let (identities, _) = match &row {
382418 Some(row) => {
383419 #[derive(Deserialize)]
384420 struct Linked {
419455 };
420456 Ok(SsoSettings {
421457 slug: slug.to_owned(),
458+ workspace_id: workspace_id.to_owned(),
422459 connection: row.as_ref().map(ConnectionRow::view),
423460 required: self.require_sso_of(workspace_id).await?,
424461 you_linked,
441478
442479 /// `sso_summary`: whether a workspace has single sign-on, for anyone.
443480 pub async fn sso_summary(&self, a: SlugArgs) -> Result<SsoSummary> {
444− let Some(workspace_id) = self.workspace_id_of(&a.slug.to_lowercase()).await? else {
481+ let Some((workspace_id, _)) = self.sso_workspace(&a.slug).await? else {
445482 return Ok(SsoSummary::default());
446483 };
447484 let Some(row) = self.connection_where("workspace_id", &workspace_id).await? else {
448485 return Ok(SsoSummary::default());
449486 };
450487 Ok(SsoSummary {
488+ workspace_id: workspace_id.clone(),
451489 enabled: true,
452490 protocol: Some(row.protocol()),
453491 required: self.require_sso_of(&workspace_id).await?,
538576 let message = match (&before, changed_provider) {
539577 (None, _) => format!("Set up single sign-on over {}", protocol_name(a.protocol)),
540578 (Some(_), true) => format!("Moved single sign-on to another identity provider ({}), unlinking everyone", protocol_name(a.protocol)),
541− (Some(_), false) => "Changed single sign-on's settings".to_owned(),
579+ (Some(before), false) => {
580+ let was = before.config();
581+ let mut changes: Vec<String> = Vec::new();
582+ if was.certificates != config.certificates {
583+ changes.push("its signing certificates".to_owned());
584+ }
585+ if was.client_id != config.client_id {
586+ changes.push("its client id".to_owned());
587+ }
588+ if a.client_secret.as_deref().is_some_and(|s| !s.trim().is_empty()) {
589+ changes.push("its client secret".to_owned());
590+ }
591+ if (before.allow_join != 0) != a.allow_join {
592+ changes.push(if a.allow_join { "letting its people join".to_owned() } else { "no longer letting its people join".to_owned() });
593+ }
594+ if was.join_domains != config.join_domains {
595+ changes.push(format!("who may join ({})", if config.join_domains.is_empty() { "anyone".to_owned() } else { config.join_domains.join(", ") }));
596+ }
597+ if changes.is_empty() { "Changed single sign-on's settings".to_owned() } else { format!("Changed single sign-on: {}", changes.join(", ")) }
598+ }
542599 };
543600 self.audit_workspace(&a.actor, if before.is_none() { "sso.created" } else { "sso.updated" }, &slug, surface, message).await;
544601 self.announce_workspace(&workspace_id, &slug, Some(&a.actor.id)).await;
672729 if !a.user.verified {
673730 return Ok(Outcome::fail(FailureCode::Forbidden, "Confirm your email address first."));
674731 }
675− let slug = a.slug.trim().to_lowercase();
676− let Some(workspace_id) = self.workspace_id_of(&slug).await? else {
732+ let Some((workspace_id, slug)) = self.sso_workspace(&a.slug).await? else {
677733 return Ok(Outcome::fail(FailureCode::NotFound, "There is no workspace with that name."));
678734 };
679735 let Some(row) = self.connection_where("workspace_id", &workspace_id).await? else {
738794 (state.clone(), url, Some(state), Some(verifier), nonce)
739795 }
740796 };
797+ // SAML's response comes back cross-site, without the session cookie:
798+ // a SameSite=None cookie of its own binds it to this browser.
799+ let binding = (row.protocol() == SsoProtocol::Saml).then(|| crypto::random_hex(32));
741800 let optional = |value: Option<&str>| value.map_or(JsValue::NULL, Into::into);
801+ let browser = binding.as_deref().map(crypto::sha256_hex);
742802 self.db
743803 .prepare(format!(
744− "INSERT INTO sso_states (id, connection_id, session_id, user_id, verifier, nonce, redirect_uri, audience, next, expires_at)
745− VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, {})",
804+ "INSERT INTO sso_states (id, connection_id, session_id, user_id, verifier, nonce, redirect_uri, audience, next, browser, expires_at)
805+ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, {})",
746806 sql_after(STATE_TTL_SECONDS)
747807 ))
748808 .bind(&[
755815 a.redirect_uri.as_str().into(),
756816 optional(a.entity_id.as_deref()),
757817 a.next.as_str().into(),
818+ optional(browser.as_deref()),
758819 ])?
759820 .run()
760821 .await?;
765826 self.db.prepare(format!("DELETE FROM sso_sessions WHERE expires_at < {SQL_NOW}")),
766827 ])
767828 .await?;
768− Ok(Outcome::Ok(SsoStart { url, state }))
829+ Ok(Outcome::Ok(SsoStart { url, state, binding }))
769830 }
770831
771832 async fn endpoints_of(&self, protocol: SsoProtocol, config: &SsoConfig) -> std::result::Result<oauth::Endpoints, String> {
782843 }
783844 }
784845
785− /// Spends a state row, once, and finds its connection, which must be
786− /// `slug`'s.
787− async fn spend_state(&self, key: &str, slug: &str) -> Result<Option<(StateRow, ConnectionRow)>> {
846+ /// Spends a state row, once, and finds its connection, which must be the
847+ /// workspace's that `workspace` names (by id or name), and that
848+ /// workspace's name now.
849+ async fn spend_state(&self, key: &str, workspace: &str) -> Result<Option<(StateRow, ConnectionRow, String)>> {
788850 let state = self
789851 .db
790852 .prepare(format!("DELETE FROM sso_states WHERE id = ? AND expires_at > {SQL_NOW} RETURNING *"))
797859 let Some(row) = self.connection_where("id", &state.connection_id).await? else {
798860 return Ok(None);
799861 };
800− let workspace = self.workspace_id_of(&slug.trim().to_lowercase()).await?;
801− Ok((workspace.as_deref() == Some(row.workspace_id.as_str())).then_some((state, row)))
862+ let Some((workspace_id, slug)) = self.sso_workspace(workspace).await? else {
863+ return Ok(None);
864+ };
865+ Ok((workspace_id == row.workspace_id).then_some((state, row, slug)))
802866 }
803867
804868 /// `sso_finish`: see [`SsoFinishArgs`].
805869 pub async fn sso_finish(&self, a: SsoFinishArgs) -> Result<Outcome<SsoFinished>> {
806− let Some((state, row)) = self.spend_state(&a.state, &a.slug).await? else {
870+ let Some((state, row, slug)) = self.spend_state(&a.state, &a.slug).await? else {
807871 return Ok(Outcome::fail(FailureCode::Invalid, EXPIRED));
808872 };
809873 if crypto::sha256_hex(&a.session_token) != state.session_id {
858922 }
859923 };
860924 match said.await {
861− Ok(said) => self.complete(&row, &state, &a.slug, &said).await,
925+ Ok(said) => self.complete(&row, &state, &slug, &said).await,
862926 Err(why) => Ok(Outcome::fail(FailureCode::Invalid, why)),
863927 }
864928 }
871935 "Start single sign-on from g1t: sign-ins started at the identity provider are not accepted.",
872936 ));
873937 };
874− let Some((state, row)) = self.spend_state(&request_id, &a.slug).await? else {
938+ let Some((state, row, slug)) = self.spend_state(&request_id, &a.slug).await? else {
875939 return Ok(Outcome::fail(FailureCode::Invalid, EXPIRED));
876940 };
941+ // Only the browser that set off: its binding cookie came back with
942+ // the response. Otherwise someone could send their own request to
943+ // another person, whose provider would answer it for them.
944+ let bound = match (&state.browser, a.binding.as_deref()) {
945+ (Some(expected), Some(binding)) => crypto::sha256_hex(binding) == *expected,
946+ _ => false,
947+ };
948+ if !bound {
949+ return Ok(Outcome::fail(FailureCode::Invalid, "This sign-in did not start in this browser. Start again from g1t."));
950+ }
877951 let config = row.config();
878952 let certificates: Vec<Vec<u8>> = config.certificates.iter().filter_map(|der| STANDARD.decode(der).ok()).collect();
879953 let expect = saml::Expect {
886960 };
887961 match saml::verify_response(&a.saml_response, &expect) {
888962 // The provider signed the assertion, address and all.
889− Ok(asserted) => self.complete(&row, &state, &a.slug, &Said::new(asserted.subject, asserted.email, true)).await,
963+ Ok(asserted) => self.complete(&row, &state, &slug, &Said::new(asserted.subject, asserted.email, true)).await,
890964 Err(why) => Ok(Outcome::fail(FailureCode::Invalid, why)),
891965 }
892966 }
905979 struct Linked {
906980 user_id: String,
907981 subject: String,
982+ ids: String,
908983 }
909984 let linked = self
910985 .db
911− .prepare("SELECT user_id, subject FROM sso_identities WHERE connection_id = ?1 AND (subject = ?2 OR user_id = ?3)")
986+ .prepare("SELECT user_id, subject, ids FROM sso_identities WHERE connection_id = ?1 AND (subject = ?2 OR user_id = ?3)")
912987 .bind(&[row.id.as_str().into(), subject.into(), user_id.into()])?
913988 .all()
914989 .await?
919994 format!("That account at {slug}'s identity provider is linked to another g1t account. An owner of {slug} can unlink it."),
920995 ));
921996 }
922− if linked.iter().any(|l| l.user_id == user_id && l.subject != subject) {
923− return Ok(Outcome::fail(
924− FailureCode::Conflict,
925− format!("Your g1t account is linked to another account at {slug}'s identity provider. Sign in with that one, or ask an owner of {slug} to unlink it."),
926− ));
997+ // Linked under another subject: the same person when another stable
998+ // id still matches (Entra's object id, after a new app
999+ // registration); otherwise someone else at the provider.
1000+ if let Some(other) = linked.iter().find(|l| l.user_id == user_id && l.subject != subject) {
1001+ let mut stored: Vec<String> = serde_json::from_str(&other.ids).unwrap_or_default();
1002+ stored.push(other.subject.clone());
1003+ if !same_person(&stored, &said.ids) {
1004+ return Ok(Outcome::fail(
1005+ FailureCode::Conflict,
1006+ format!("Your g1t account is linked to another account at {slug}'s identity provider. Sign in with that one, or ask an owner of {slug} to unlink it."),
1007+ ));
1008+ }
9271009 }
9281010 // With SCIM on, only someone the provider has assigned, matched
9291011 // without doubt, and not as another g1t account (provisioning.rs).
9741056 // Refusals done: write.
9751057 let now = rfc3339(now_ms());
9761058 let email_value = said.email.as_deref().map_or(JsValue::NULL, Into::into);
1059+ let ids = serde_json::to_string(&said.ids).unwrap_or_else(|_| "[]".to_owned());
1060+ let trusted = said.trusted_email().map_or(JsValue::NULL, Into::into);
9771061 if linked.is_empty() {
9781062 self.db
9791063 .prepare(
980− "INSERT INTO sso_identities (connection_id, user_id, subject, email, created_at, last_used_at)
981− VALUES (?, ?, ?, ?, ?, ?)",
1064+ "INSERT INTO sso_identities (connection_id, user_id, subject, email, ids, trusted_email, created_at, last_used_at)
1065+ VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
9821066 )
983− .bind(&[row.id.as_str().into(), user_id.into(), subject.into(), email_value, now.as_str().into(), now.as_str().into()])?
1067+ .bind(&[
1068+ row.id.as_str().into(),
1069+ user_id.into(),
1070+ subject.into(),
1071+ email_value,
1072+ ids.as_str().into(),
1073+ trusted,
1074+ now.as_str().into(),
1075+ now.as_str().into(),
1076+ ])?
9841077 .run()
9851078 .await?;
9861079 self.audit_workspace(&person, "sso.identity_linked", &slug, Surface::Web, format!("Linked to {subject} at the single sign-on provider"))
9871080 .await;
9881081 } else {
9891082 self.db
990− .prepare("UPDATE sso_identities SET last_used_at = ?, email = COALESCE(?, email) WHERE connection_id = ? AND user_id = ?")
991− .bind(&[now.as_str().into(), email_value, row.id.as_str().into(), user_id.into()])?
1083+ .prepare(
1084+ "UPDATE sso_identities SET subject = ?, ids = ?, trusted_email = ?, last_used_at = ?, email = COALESCE(?, email)
1085+ WHERE connection_id = ? AND user_id = ?",
1086+ )
1087+ .bind(&[subject.into(), ids.as_str().into(), trusted, now.as_str().into(), email_value, row.id.as_str().into(), user_id.into()])?
9921088 .run()
9931089 .await?;
9941090 }
9991095 .run()
10001096 .await?;
10011097 }
1098+ // With SCIM, only while the assignment is still active: a
1099+ // deprovisioning that lands mid-sign-in wins.
1100+ let still_assigned = "(?4 IS NULL OR EXISTS (SELECT 1 FROM scim_users WHERE id = ?4 AND active = 1))";
1101+ let assignment = provisioned.as_deref().map_or(JsValue::NULL, Into::into);
10021102 let mut joined = false;
10031103 if !member {
10041104 joined = self
10051105 .db
1006− .prepare(
1106+ .prepare(format!(
10071107 "INSERT OR IGNORE INTO workspace_members (workspace_id, user_id, role, created_at)
1008− VALUES (?, ?, 'member', ?) RETURNING user_id",
1009− )
1010− .bind(&[row.workspace_id.as_str().into(), user_id.into(), now.as_str().into()])?
1108+ SELECT ?1, ?2, 'member', ?3 WHERE {still_assigned} RETURNING user_id"
1109+ ))
1110+ .bind(&[row.workspace_id.as_str().into(), user_id.into(), now.as_str().into(), assignment.clone()])?
10111111 .first::<Value>(None)
10121112 .await?
10131113 .is_some();
10201120 self.db
10211121 .prepare(format!(
10221122 "INSERT INTO sso_sessions (session_id, workspace_id, expires_at)
1023− SELECT ?1, ?2, {} WHERE EXISTS (SELECT 1 FROM sessions WHERE id = ?1 AND user_id = ?3 AND expires_at > {SQL_NOW})
1123+ SELECT ?1, ?2, {} WHERE EXISTS (SELECT 1 FROM sessions WHERE id = ?1 AND user_id = ?3 AND expires_at > {SQL_NOW}) AND {still_assigned}
10241124 ON CONFLICT (session_id, workspace_id) DO UPDATE SET expires_at = excluded.expires_at",
10251125 sql_after(SSO_SESSION_SECONDS)
10261126 ))
1027− .bind(&[state.session_id.as_str().into(), row.workspace_id.as_str().into(), user_id.into()])?
1127+ .bind(&[state.session_id.as_str().into(), row.workspace_id.as_str().into(), user_id.into(), assignment])?
10281128 .run()
10291129 .await?;
10301130 let first = self
11951295 fn entra_gives_its_object_id_and_a_members_principal_name() {
11961296 let claims = serde_json::json!({ "sub": "pairwise", "oid": "11111111-2222", "preferred_username": "Ada@Contoso.com", "email": "ada@contoso.com" });
11971297 let said = said_by_oidc(&claims, "https://login.microsoftonline.com/tid/v2.0").unwrap();
1198− assert_eq!(said.ids, vec!["pairwise".to_owned(), "11111111-2222".to_owned(), "ada@contoso.com".to_owned()]);
1298+ // The object id, never the mutable principal name.
1299+ assert_eq!(said.ids, vec!["pairwise".to_owned(), "11111111-2222".to_owned()]);
11991300 // Entra sends no email_verified: its address is never trusted.
12001301 assert_eq!(said.trusted_email(), None);
1201− let guest = serde_json::json!({ "sub": "p", "preferred_username": "eve_gmail.com#EXT#@contoso.onmicrosoft.com" });
1202− assert_eq!(said_by_oidc(&guest, "https://login.microsoftonline.com/tid/v2.0").unwrap().ids, vec!["p".to_owned()]);
1302+ // A new app registration: another pairwise subject, the same object id.
1303+ assert!(same_person(&["old-pairwise".to_owned(), "11111111-2222".to_owned()], &said.ids));
1304+ assert!(!same_person(&["old-pairwise".to_owned()], &said.ids));
1305+ let stringly = serde_json::json!({ "sub": "1", "email": "a@b.co", "email_verified": "true" });
1306+ assert_eq!(said_by_oidc(&stringly, "https://idp.test").unwrap().trusted_email(), Some("a@b.co"));
12031307 let okta = serde_json::json!({ "sub": "00u1", "oid": "x", "email": "ada@acme.com", "email_verified": true });
12041308 let said = said_by_oidc(&okta, "https://acme.okta.com").unwrap();
12051309 assert_eq!(said.ids, vec!["00u1".to_owned()]);
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…
Loading the diff…

Comments

Sign in to comment.