diff --git a/dashboard/src/guest.rs b/dashboard/src/guest.rs index ceb776e7339f7cee57acdecba787a7720664bfa5..95ab45d2bdbabf751765b04a972b080debb1cb5b 100644 --- a/dashboard/src/guest.rs +++ b/dashboard/src/guest.rs @@ -291,20 +291,6 @@ fn signed_claims( Ok(claims) } -pub(crate) fn astheno_profile(value: &str) -> Option { - let url = url::Url::parse(value).ok()?; - (url.origin().ascii_serialization() == ASTHENO - && url.username().is_empty() - && url.password().is_none() - && url.query().is_none() - && url.fragment().is_none() - && url - .path() - .strip_prefix("/user/") - .is_some_and(|id| !id.is_empty() && !id.contains('/'))) - .then(|| url.into()) -} - async fn exchange( http: &reqwest::Client, provider: &str, @@ -313,7 +299,7 @@ async fn exchange( code: &str, callback: &str, flow: &Value, -) -> Result<(String, String, Option, Option)> { +) -> Result<(String, String, Option)> { let form = [ ("client_id", client), ("client_secret", secret), @@ -370,7 +356,7 @@ async fn exchange( "GitHub couldn't verify your account. Return to Shale and try again.", ) })?; - return Ok((id.to_string(), login.to_owned(), None, None)); + return Ok((id.to_string(), login.to_owned(), None)); } let mut form = form.to_vec(); form.push(("state", "none")); @@ -455,13 +441,7 @@ async fn exchange( && url.password().is_none() }) .map(String::from); - let public_profile = profile["profile"].as_str().and_then(astheno_profile); - Ok(( - string(&profile["sub"]).to_owned(), - name, - picture, - public_profile, - )) + Ok((string(&profile["sub"]).to_owned(), name, picture)) } fn account( @@ -470,12 +450,7 @@ fn account( subject: &str, name: &str, picture: Option<&str>, - public_profile: Option<&str>, ) -> Result { - let mut attributes = json!({"picture":picture.map(|picture| vec![picture])}); - if let Some(profile) = public_profile { - attributes["profile"] = json!([profile]); - } let mut db = auth.db.lock().unwrap(); let tx = db.transaction()?; let existing: Option = tx @@ -495,10 +470,7 @@ fn account( } tx.execute( "UPDATE users SET profile=json_patch(profile,?) WHERE id=?", - sql![ - json!({"firstName":name,"attributes":attributes}).to_string(), - id - ], + sql![json!({"firstName":name,"attributes":{"picture":picture.map(|picture| vec![picture])}}).to_string(), id], )?; tx.commit()?; return Ok(id); @@ -509,11 +481,10 @@ fn account( } else { mcp::hash(subject)[..24].to_owned() }; - let mut profile = json!({"kind":"guest","guestProvider":provider,"username":format!("guest-{provider}-{suffix}"),"enabled":true,"email":null,"emailVerified":false,"firstName":name,"lastName":null,"requiredActions":[],"createdTimestamp":(now()*1000.0) as i64}); - if picture.is_none() { - attributes.as_object_mut().unwrap().remove("picture"); + let mut profile = json!({"kind":"guest","guestProvider":provider,"username":format!("guest-{provider}-{suffix}"),"enabled":true,"email":null,"emailVerified":false,"firstName":name,"lastName":null,"requiredActions":[],"attributes":{},"createdTimestamp":(now()*1000.0) as i64}); + if let Some(picture) = picture { + profile["attributes"]["picture"] = json!([picture]); } - profile["attributes"] = attributes; tx.execute( "INSERT INTO users(id,profile) VALUES (?,?)", sql![id, profile.to_string()], @@ -652,16 +623,9 @@ async fn handle(app: &App, request: Request) -> Result { headers }) .build()?; - let (subject, name, picture, public_profile) = + let (subject, name, picture) = exchange(&http, provider, &client, &secret, code, &callback, &flow).await?; - let id = account( - auth, - provider, - &subject, - &name, - picture.as_deref(), - public_profile.as_deref(), - )?; + let id = account(auth, provider, &subject, &name, picture.as_deref())?; auth.create_session(&id, "dashboard", headers, None) } .await; @@ -826,22 +790,6 @@ mod tests { } } - #[test] - fn public_astheno_profiles_stay_on_the_identity_origin() { - let profile = "https://identity.astheno.software/user/00653PKPFWTHWVX7K6NZ06ZW79"; - assert_eq!(astheno_profile(profile).as_deref(), Some(profile)); - for value in [ - "http://identity.astheno.software/user/id", - "https://other.test/user/id", - "https://identity.astheno.software/user/", - "https://identity.astheno.software/user/id/extra", - "https://identity.astheno.software/user/id?query=yes", - "https://user@identity.astheno.software/user/id", - ] { - assert!(astheno_profile(value).is_none()); - } - } - #[test] fn identities_never_link_by_name_or_email_and_cannot_gain_credentials_or_groups() { let path = std::env::temp_dir().join(format!("guest-test-{}", uuid::Uuid::new_v4())); @@ -858,15 +806,14 @@ mod tests { db.execute("INSERT INTO roles VALUES ('admin','infra-admin') ON CONFLICT(name) DO UPDATE SET id=excluded.id", []) .unwrap(); } - let first = account(&auth, "github", "123", "clover", None, None).unwrap(); - let repeat = account(&auth, "github", "123", "renamed", None, None).unwrap(); + let first = account(&auth, "github", "123", "clover", None).unwrap(); + let repeat = account(&auth, "github", "123", "renamed", None).unwrap(); let other = account( &auth, "astheno", "123", "clover", Some("https://identity.astheno.software/avatar/123"), - Some("https://identity.astheno.software/user/public-id"), ) .unwrap(); assert_eq!(first, repeat); @@ -876,11 +823,7 @@ mod tests { auth::user(&auth.db.lock().unwrap(), &other).unwrap()["attributes"]["picture"][0], "https://identity.astheno.software/avatar/123" ); - account(&auth, "astheno", "123", "clover", None, None).unwrap(); - assert_eq!( - auth::user(&auth.db.lock().unwrap(), &other).unwrap()["attributes"]["profile"][0], - "https://identity.astheno.software/user/public-id" - ); + account(&auth, "astheno", "123", "clover", None).unwrap(); assert!( auth::user(&auth.db.lock().unwrap(), &other).unwrap()["attributes"] .get("picture") @@ -911,7 +854,7 @@ mod tests { ) .unwrap(); } - assert!(account(&auth, "github", "123", "clover", None, None).is_err()); + assert!(account(&auth, "github", "123", "clover", None).is_err()); assert!( auth.create_session(&other, "file", &HeaderMap::new(), None) .is_err() diff --git a/dashboard/src/shale.rs b/dashboard/src/shale.rs index 39d5dfa82bddc47abd893ea4161cac0e9c9dff65..39edd511b4aa7caa9f8411eecb41ae8c92109fc5 100644 --- a/dashboard/src/shale.rs +++ b/dashboard/src/shale.rs @@ -326,7 +326,7 @@ fn issue(html: &str, repository: &str, id: Option) -> Result { ) })?; let author = comment - .select(&Selector::parse(".n-card__header a[href^='/~'], .n-card__header a[href^='https://github.com/'], .n-card__header a[href^='https://identity.astheno.software/user/']").unwrap()) + .select(&Selector::parse(".n-card__header a[href^='/~'], .n-card__header a[href^='https://github.com/'], .n-card__header a:not([href]).va-middle-childs").unwrap()) .next() .map(text); let time = comment @@ -937,7 +937,7 @@ mod tests { } #[test] fn issue_comment_authors_include_external_guest_profiles() { - let page = "

#3Title

Done
"; + let page = "

#3Title

Done
"; let parsed = issue(page, "owned", Some(3)).unwrap(); assert_eq!(parsed["comments"][0]["author"], "paperclover"); assert_eq!(parsed["comments"][1]["author"], "Astheno user"); diff --git a/dashboard/src/shale_page.rs b/dashboard/src/shale_page.rs index 3622018506b6bb6a0d9472978b0b003a8231028b..b06e222c693f5651352679c78428b8e984e95bc4 100644 --- a/dashboard/src/shale_page.rs +++ b/dashboard/src/shale_page.rs @@ -4,14 +4,14 @@ use lol_html::{RewriteStrSettings, element, html_content::ContentType, text}; struct Profile { name: String, - url: String, + url: Option, icon: &'static str, picture: Option, } fn profiles(auth: &auth::Store) -> Result> { let db = auth.db.lock().unwrap(); - let mut query = db.prepare("SELECT json_extract(profile,'$.username'),coalesce(json_extract(profile,'$.firstName'),''),provider,subject,json_extract(profile,'$.attributes.picture[0]'),json_extract(profile,'$.attributes.profile[0]') FROM users JOIN external_identities ON user_id=users.id WHERE json_extract(profile,'$.kind')='guest'")?; + let mut query = db.prepare("SELECT json_extract(profile,'$.username'),coalesce(json_extract(profile,'$.firstName'),''),provider,subject,json_extract(profile,'$.attributes.picture[0]') FROM users JOIN external_identities ON user_id=users.id WHERE json_extract(profile,'$.kind')='guest'")?; let rows = query.query_map([], |row| { Ok(( row.get::<_, String>(0)?, @@ -19,24 +19,20 @@ fn profiles(auth: &auth::Store) -> Result> { row.get::<_, String>(2)?, row.get::<_, String>(3)?, row.get::<_, Option>(4)?, - row.get::<_, Option>(5)?, )) })?; let mut profiles = HashMap::new(); for row in rows { - let (username, name, provider, subject, picture, public_profile) = row?; + let (username, name, provider, subject, picture) = row?; if name.is_empty() { continue; } let (url, icon) = match provider.as_str() { "github" => { if matches!(name.as_str(), "." | "..") { continue; } let mut url = url::Url::parse("https://github.com/")?; url.path_segments_mut().unwrap().pop_if_empty().push(&name); - (String::from(url), include_str!("../web/sso/github.svg")) - } - "astheno" => { - let Some(url) = public_profile.as_deref().and_then(guest::astheno_profile) else { continue; }; - (url, include_str!("astheno.svg")) + (Some(String::from(url)), include_str!("../web/sso/github.svg")) } + "astheno" => (None, include_str!("astheno.svg")), _ => continue, }; profiles.insert( @@ -79,19 +75,28 @@ fn rewrite(html: &str, origin: &url::Url, profiles: &HashMap) - let Some(profile) = profile else { return Ok(()); }; let closing = active.clone(); element.on_end_tag(lol_html::end_tag!(move |_| { closing.set(None); Ok(()) }))?; - element.set_attribute("href", &profile.url)?; - element.set_attribute("target", "_blank")?; - element.set_attribute("rel", "noreferrer")?; + if let Some(url) = &profile.url { + element.set_attribute("href", url)?; + element.set_attribute("target", "_blank")?; + element.set_attribute("rel", "noreferrer")?; + } else { + for attribute in ["href", "target", "rel", "tabindex"] { element.remove_attribute(attribute); } + } let style = element.get_attribute("style").unwrap_or_default(); - element.set_attribute("style", &format!("{style};text-decoration-line:underline;text-decoration-color:currentColor"))?; + let cursor = if profile.url.is_some() { "" } else { ";cursor:default" }; + element.set_attribute("style", &format!("{style};text-decoration:none{cursor}"))?; if let Some(picture) = &profile.picture { let picture = picture.replace('&', "&").replace('"', """).replace('<', "<"); element.prepend(&format!("\"\""), ContentType::Html); } let icon = profile.icon.trim().replace("", ContentType::Html); + } element.append(&profile.name, ContentType::Text); + if profile.url.is_some() { element.append("", ContentType::Html); } Ok(()) })) .append_element_content_handler(element!("a[href] img, a[href] span", move |child| { @@ -170,12 +175,12 @@ pub async fn proxy(app: Arc, request: Request) -> Result { let origin = url::Url::parse(&format!("https://{host}{uri}"))?; let profiles = profiles(&app.auth)?; if matches!(parts.method, Method::GET | Method::HEAD) - && let Some(profile) = account_path(uri.path()).and_then(|name| profiles.get(name)) + && let Some(url) = account_path(uri.path()).and_then(|name| profiles.get(name)).and_then(|profile| profile.url.as_deref()) { return Ok(( StatusCode::FOUND, [ - ("location", profile.url.as_str()), + ("location", url), ("cache-control", "no-store"), ("referrer-policy", "no-referrer"), ], @@ -275,7 +280,7 @@ mod tests { "guest-github-123".into(), Profile { name: "<&\"clover".into(), - url: "https://github.com/clover".into(), + url: Some("https://github.com/clover".into()), icon: include_str!("../web/sso/github.svg"), picture: Some("https://avatars.githubusercontent.com/u/123?s=64".into()), }, @@ -284,7 +289,7 @@ mod tests { "guest-astheno-abc".into(), Profile { name: "Astheno user".into(), - url: "https://identity.astheno.software/user/123".into(), + url: None, icon: include_str!("astheno.svg"), picture: None, }, @@ -293,10 +298,16 @@ mod tests { let html = r#"~guest-github-123avatarguestunknownexternalrepo
~guest-github-123
"#; let rewritten = rewrite(html, &origin, &profiles).unwrap(); let page = Html::parse_document(&rewritten); - let links: Vec<_> = page.select(&Selector::parse("a").unwrap()).collect(); + let links: Vec<_> = page.select(&Selector::parse("body > a").unwrap()).collect(); + assert_eq!(links[0].attr("target"), Some("_blank")); + assert_eq!(links[0].attr("rel"), Some("noreferrer")); + assert!(links[0].attr("style").unwrap().contains("text-decoration:none")); + assert!(links[0].select(&Selector::parse("span[style]").unwrap()).next().unwrap().attr("style").unwrap().contains("text-decoration:underline")); + assert_eq!(links[1].value().name(), "a"); + assert_eq!(links[1].attr("target"), None); + assert_eq!(links[1].attr("rel"), None); + assert_eq!(links[1].text().collect::().trim(), "Astheno user"); for link in &links[..2] { - assert_eq!(link.attr("target"), Some("_blank")); - assert_eq!(link.attr("rel"), Some("noreferrer")); assert_eq!( link.select(&Selector::parse("svg[aria-hidden=true]").unwrap()) .count(), @@ -322,10 +333,7 @@ mod tests { ); assert_eq!(links[0].text().collect::().trim(), "<&\"clover"); assert_eq!(links[0].attr("class"), Some("usa-nav-link")); - assert_eq!( - links[1].attr("href"), - Some("https://identity.astheno.software/user/123") - ); + assert_eq!(links[1].attr("href"), None); for link in &links[2..] { assert_eq!(link.attr("target"), None); } diff --git a/tools/dashboard-shale-page-test.py b/tools/dashboard-shale-page-test.py index 9f2bcc2404d069293c994e8858fde93ebb3b81ff..783dd59847725d14dd333dbb3e18377ce846f527 100644 --- a/tools/dashboard-shale-page-test.py +++ b/tools/dashboard-shale-page-test.py @@ -57,6 +57,9 @@ def main(): return self.respond(blob, 'application/octet-stream') if self.path == '/large': return self.respond(b' ' * (9 * 1024 * 1024), 'text/html') + if self.path == '/-/login': + callback = 'https://' + self.headers['Host'] + '/-/callback' + return self.respond(b'', 'text/plain', 302, [('Location', 'https://snowglobe.studio.test/auth/oidc/authorize?redirect_uri=' + urllib.parse.quote(callback, safe=''))]) if self.path == '/redirect': return self.respond(b'', 'text/plain', 302, [('Location', '/snowbound/issues/26'), ('Set-Cookie', 'SessionID=new; Path=/; HttpOnly'), ('Set-Cookie', 'other=kept; Path=/')]) if self.path.startswith('/-/') and args.page: @@ -122,11 +125,9 @@ def main(): for provider, subject, suffix, name in [('github', '24465214', '24465214', 'paperclover'), ('astheno', 'pairwise-astheno-subject', 'abc', 'Astheno user'), ('github', '777', '777', None), ('astheno', 'unmapped-pairwise-subject', 'unknown', 'Unmapped guest')]: username = f'guest-{provider}-{suffix}' profile = {'kind': 'guest', 'enabled': True, 'username': username, 'guestProvider': provider, 'firstName': name} - if provider == 'astheno' and suffix == 'abc': - profile['attributes'] = {'profile': ['https://identity.astheno.software/user/00653DG7HZ7MCGTW2BPG7RMGQB']} db.execute('INSERT INTO users(id,profile) VALUES (?,?)', [username, json.dumps(profile)]) db.execute('INSERT INTO external_identities VALUES (?,?,?)', [provider, subject, username]) - os.environ.update(STUDIO_DOMAIN='studio.test', STUDIO_DASHBOARD_PORT=str(dashboard_port), STUDIO_PROXY_TOKEN_FILE=str(token)) + os.environ.update(STUDIO_DOMAIN='studio.test', STUDIO_DASHBOARD_PORT=str(dashboard_port), STUDIO_PROXY_TOKEN_FILE=str(token), STUDIO_INTERNAL_PORT=str(internal_port)) router.ROUTE_DIR = str(root / 'routes') Path(router.ROUTE_DIR).mkdir() (Path(router.ROUTE_DIR) / 'shale.json').write_text(json.dumps({'headHtml': {'shale.studio.test': {'/snowbound/*': ''}}})) @@ -153,10 +154,9 @@ def main(): preview_port = port() config = '{\n admin off\n auto_https off\n}\n' + local_site('shale.studio.test', gateway_port) + '\n' + local_site('shale-preview-12345678.studio.test', preview_port) - config += f'\nhttps://localhost:{internal_port} {{\n tls {certificate} {gateway_key}\n @trusted header Studio-Proxy-Token {proof}\n handle @trusted {{\n request_header -Studio-Proxy-Token\n' - for service in ['shale', 'shale-preview-12345678']: - config += f' handle_path /services/{service}/* {{\n reverse_proxy 127.0.0.1:{fixture.server_port}\n }}\n' - config += ' }\n handle {\n respond 403\n }\n}\n' + internal_host = f'dashboard.internal.studio.test:{internal_port}' + start = rendered.index(internal_host + ' {') + config += '\n' + rendered[start:].replace(internal_host + ' {', f'https://localhost:{internal_port} {{', 1).replace(' tls internal\n', f' tls {certificate} {gateway_key}\n', 1) config_path = root / 'Caddyfile' config_path.write_text(config) subprocess.run([str(args.caddy), 'validate', '--config', str(config_path), '--adapter', 'caddyfile'], check=True, capture_output=True, env=caddy_env) @@ -186,26 +186,32 @@ def main(): time.sleep(.05) assert status == 200, (status, body, (root / 'dashboard.log').read_text(), (root / 'caddy.log').read_text()) links = Links(body.decode()).links - for url in ['https://github.com/paperclover', 'https://identity.astheno.software/user/00653DG7HZ7MCGTW2BPG7RMGQB']: - link = next(link for link in links if link['href'] == url) - assert link['target'] == '_blank' and link['rel'] == 'noreferrer', link - assert any(link['href'] == '/~guest-github-unknown' for link in links) - assert any(link['href'] == '/~guest-github-24465214/repo' for link in links) + link = next(link for link in links if link.get('href') == 'https://github.com/paperclover') + assert link['target'] == '_blank' and link['rel'] == 'noreferrer', link + assert any(link.get('href') == '/~guest-github-unknown' for link in links) + assert any(link.get('href') == '/~guest-github-24465214/repo' for link in links) + assert b'Astheno user' in body and not any(link.get('href') == '/~guest-astheno-abc' for link in links) + assert b'identity.astheno.software/user/' not in body and b'pairwise-astheno-subject' not in body assert b'https://avatars.githubusercontent.com/u/24465214?s=64' in body assert b'rewrite-test' in body assert headers['Cache-Control'] == 'no-store' and headers.get('ETag') is None and headers.get('Last-Modified') is None assert observations[-1][1] == '/snowbound/issues/26?query=kept', observations[-1] + assert observations[-1][2]['host'] == 'shale.studio.test', observations[-1] assert observations[-1][2]['cookie'] == 'SessionID=preserved', observations[-1] assert not any(key.lower().startswith('studio-') for key in observations[-1][2]), observations[-1] - for path, target in [('/~guest-github-24465214', 'https://github.com/paperclover'), ('/~guest-astheno-abc/', 'https://identity.astheno.software/user/00653DG7HZ7MCGTW2BPG7RMGQB')]: - status, headers, body = request(path) - assert status == 302 and headers['Location'] == target and headers['Referrer-Policy'] == 'no-referrer', (path, status, dict(headers), observations[-1][:2]) + status, headers, body = request('/~guest-github-24465214') + assert status == 302 and headers['Location'] == 'https://github.com/paperclover' and headers['Referrer-Policy'] == 'no-referrer' + assert request('/~guest-astheno-abc/')[0] == 200 + assert observations[-1][1] == '/~guest-astheno-abc/' assert request('/~guest-astheno-unknown')[0] == 200 + status, headers, body = request('/-/login') + assert status == 302 and urllib.parse.parse_qs(urllib.parse.urlparse(headers['Location']).query)['redirect_uri'] == ['https://shale.studio.test/-/callback'] status, headers, body = request('/redirect') assert status == 302 and headers['Location'] == '/snowbound/issues/26' and len(headers.get_all('Set-Cookie')) == 2 with opener.open(f'http://127.0.0.1:{preview_port}/snowbound/issues/26') as response: assert response.status == 200 - assert any(link['href'] == 'https://github.com/paperclover' for link in Links(response.read().decode()).links) + assert observations[-1][2]['host'] == 'shale-preview-12345678.studio.test' + assert any(link.get('href') == 'https://github.com/paperclover' for link in Links(response.read().decode()).links) assert b'/~guest-github-24465214' in request('/repo/info/refs')[2] assert request('/binary')[2] == blob assert len(request('/large')[2]) == 9 * 1024 * 1024 diff --git a/tools/router.py b/tools/router.py index 46dc62c7713a2a7460810853a952cfc0713e9922..00707cb860dcfc7e8d594a5561e92fcfacc38770 100644 --- a/tools/router.py +++ b/tools/router.py @@ -24,13 +24,13 @@ def nomad(path, token): return json.load(response) -def proxy(upstreams, indent, uncompressed=False, upstream_host=False): +def proxy(upstreams, indent, uncompressed=False, host=None): lines = [f"{indent}reverse_proxy {upstreams} {{", f"{indent} lb_try_duration 5s", f"{indent} fail_duration 30s"] if uncompressed: lines.append(f"{indent} header_up Accept-Encoding identity") - if upstream_host: - lines.append(f"{indent} header_up Host {{upstream_hostport}}") + if host: + lines.append(f"{indent} header_up Host {host}") return [*lines, f"{indent}}}"] @@ -357,10 +357,11 @@ def render(token): f" @dashboard header Studio-Proxy-Token {dashboard_proof}", " handle @dashboard {", " request_header -Studio-Proxy-Token", " request_header -User-Name", " request_header -User-Groups", - " handle_path /nomad/* {", *proxy("127.0.0.1:4646", " ", upstream_host=True), " }"] + " handle_path /nomad/* {", *proxy("127.0.0.1:4646", " ", host="{upstream_hostport}"), " }"] for service, upstreams in sorted(internal_services.items()): + host = f"{service}.{os.environ['STUDIO_DOMAIN']}" if service == "shale" or re.fullmatch(r"shale-preview-[0-9a-f]{8}", service) else "{upstream_hostport}" lines += [f" handle_path /services/{service}/* {{", - *proxy(" ".join(sorted(upstreams)), " ", upstream_host=True), " }"] + *proxy(" ".join(sorted(upstreams)), " ", host=host), " }"] lines += [" handle {", " respond 404", " }", " }", " handle {", " respond 403", " }", "}"] return "\n".join(lines) + "\n"