diff --git a/nexus/src/app/iam.rs b/nexus/src/app/iam.rs index 2e28c9ac03b..5963dcd5129 100644 --- a/nexus/src/app/iam.rs +++ b/nexus/src/app/iam.rs @@ -93,8 +93,9 @@ impl super::Nexus { pub async fn silo_user_fetch_groups_for_self( &self, opctx: &OpContext, - ) -> ListResultVec { - self.db_datastore.silo_group_membership_for_self(opctx).await + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + self.db_datastore.silo_groups_for_self(opctx, pagparams).await } // Silo groups diff --git a/nexus/src/db/datastore/silo_group.rs b/nexus/src/db/datastore/silo_group.rs index 5d83d0af99e..0261dc55428 100644 --- a/nexus/src/db/datastore/silo_group.rs +++ b/nexus/src/db/datastore/silo_group.rs @@ -106,10 +106,11 @@ impl DataStore { .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } - pub async fn silo_group_membership_for_self( + pub async fn silo_groups_for_self( &self, opctx: &OpContext, - ) -> ListResultVec { + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { // Similar to session_hard_delete (see comment there), we do not do a // typical authz check, instead effectively encoding the policy here // that any user is allowed to fetch their own group memberships @@ -118,10 +119,12 @@ impl DataStore { .actor_required() .internal_context("fetching current user's group memberships")?; - use db::schema::silo_group_membership::dsl; - dsl::silo_group_membership - .filter(dsl::silo_user_id.eq(actor.actor_id())) - .select(SiloGroupMembership::as_returning()) + use db::schema::{silo_group as sg, silo_group_membership as sgm}; + paginated(sg::dsl::silo_group, sg::id, pagparams) + .inner_join(sgm::table.on(sgm::silo_group_id.eq(sg::id))) + .filter(sgm::silo_user_id.eq(actor.actor_id())) + .filter(sg::time_deleted.is_null()) + .select(SiloGroup::as_returning()) .get_results_async(self.pool_authorized(opctx).await?) .await .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) diff --git a/nexus/src/external_api/console_api.rs b/nexus/src/external_api/console_api.rs index 1886626bce6..1f4aead4322 100644 --- a/nexus/src/external_api/console_api.rs +++ b/nexus/src/external_api/console_api.rs @@ -28,12 +28,15 @@ use dropshot::{ endpoint, http_response_found, http_response_see_other, HttpError, HttpResponseFound, HttpResponseHeaders, HttpResponseOk, HttpResponseSeeOther, HttpResponseUpdatedNoContent, Path, Query, - RequestContext, TypedBody, + RequestContext, ResultsPage, TypedBody, }; use http::{header, Response, StatusCode}; use hyper::Body; use lazy_static::lazy_static; use mime_guess; +use omicron_common::api::external::http_pagination::{ + data_page_params_for, PaginatedById, ScanById, ScanParams, +}; use omicron_common::api::external::Error; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -592,7 +595,7 @@ pub async fn login_begin( }] pub async fn session_me( rqctx: Arc>>, -) -> Result, HttpError> { +) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.nexus; let handler = async { @@ -601,13 +604,43 @@ pub async fn session_me( // not clear what the advantage would be. let opctx = OpContext::for_external_api(&rqctx).await?; let user = nexus.silo_user_fetch_self(&opctx).await?; - let groups = nexus.silo_user_fetch_groups_for_self(&opctx).await?; - Ok(HttpResponseOk(views::SessionMe { - id: user.id(), - display_name: user.external_id, - silo_id: user.silo_id, - group_ids: groups.iter().map(|g| g.silo_group_id).collect(), - })) + Ok(HttpResponseOk(user.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Fetch the silo groups the current user belongs to +#[endpoint { + method = GET, + path = "/session/me/groups", + tags = ["hidden"], + }] +pub async fn session_me_groups( + rqctx: Arc>>, + query_params: Query, +) -> Result>, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let query = query_params.into_inner(); + let handler = async { + // We don't care about authentication method, as long as they are authed + // as _somebody_. We could restrict this to session auth only, but it's + // not clear what the advantage would be. + let opctx = OpContext::for_external_api(&rqctx).await?; + let groups = nexus + .silo_user_fetch_groups_for_self( + &opctx, + &data_page_params_for(&rqctx, &query)?, + ) + .await? + .into_iter() + .map(|d| d.into()) + .collect(); + Ok(HttpResponseOk(ScanById::results_page( + &query, + groups, + &|_, group: &views::Group| group.id, + )?)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 55a7f340944..653b6eb9fff 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -260,6 +260,7 @@ pub fn external_api() -> NexusApiDescription { api.register(console_api::logout)?; api.register(console_api::session_me)?; + api.register(console_api::session_me_groups)?; api.register(console_api::console_page)?; api.register(console_api::console_root)?; api.register(console_api::console_settings_page)?; diff --git a/nexus/tests/integration_tests/console_api.rs b/nexus/tests/integration_tests/console_api.rs index 42c93e7dafb..e220d8551ed 100644 --- a/nexus/tests/integration_tests/console_api.rs +++ b/nexus/tests/integration_tests/console_api.rs @@ -3,6 +3,7 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use dropshot::test_util::ClientTestContext; +use dropshot::ResultsPage; use http::header::HeaderName; use http::{header, method::Method, StatusCode}; use std::env::current_dir; @@ -335,16 +336,15 @@ async fn test_session_me(cptestctx: &ControlPlaneTestContext) { .execute() .await .expect("failed to get current user") - .parsed_body::() + .parsed_body::() .unwrap(); assert_eq!( priv_user, - views::SessionMe { + views::User { id: USER_TEST_PRIVILEGED.id(), display_name: USER_TEST_PRIVILEGED.external_id.clone(), silo_id: DEFAULT_SILO.id(), - group_ids: vec![], } ); @@ -353,20 +353,54 @@ async fn test_session_me(cptestctx: &ControlPlaneTestContext) { .execute() .await .expect("failed to get current user") - .parsed_body::() + .parsed_body::() .unwrap(); assert_eq!( unpriv_user, - views::SessionMe { + views::User { id: USER_TEST_UNPRIVILEGED.id(), display_name: USER_TEST_UNPRIVILEGED.external_id.clone(), silo_id: DEFAULT_SILO.id(), - group_ids: vec![], } ); } +#[nexus_test] +async fn test_session_me_groups(cptestctx: &ControlPlaneTestContext) { + let testctx = &cptestctx.external_client; + + // hitting /session/me without being logged in is a 401 + RequestBuilder::new(&testctx, Method::GET, "/session/me/groups") + .expect_status(Some(StatusCode::UNAUTHORIZED)) + .execute() + .await + .expect("failed to 401 on unauthed request"); + + // now make same request with auth + let priv_user_groups = + NexusRequest::object_get(testctx, "/session/me/groups") + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("failed to get current user") + .parsed_body::>() + .unwrap(); + + assert_eq!(priv_user_groups.items, vec![]); + + let unpriv_user_groups = + NexusRequest::object_get(testctx, "/session/me/groups") + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .expect("failed to get current user") + .parsed_body::>() + .unwrap(); + + assert_eq!(unpriv_user_groups.items, vec![]); +} + #[nexus_test] async fn test_login_redirect(cptestctx: &ControlPlaneTestContext) { let testctx = &cptestctx.external_client; diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index e3377fa4cd4..ac4834e8afc 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -1542,6 +1542,14 @@ lazy_static! { AllowedMethod::Get, ], }, + VerifyEndpoint { + url: "/session/me/groups", + visibility: Visibility::Public, + unprivileged_access: UnprivilegedAccess::ReadOnly, + allowed_methods: vec![ + AllowedMethod::Get, + ], + }, /* SSH keys */ diff --git a/nexus/tests/integration_tests/saml.rs b/nexus/tests/integration_tests/saml.rs index b3542e88767..eb2827e9070 100644 --- a/nexus/tests/integration_tests/saml.rs +++ b/nexus/tests/integration_tests/saml.rs @@ -1030,9 +1030,9 @@ async fn test_post_saml_response(cptestctx: &ControlPlaneTestContext) { assert_same_items(silo_group_names, vec!["SRE", "Admins"]); - let session_me: views::SessionMe = NexusRequest::new( + let session_me: views::User = NexusRequest::new( RequestBuilder::new(client, Method::GET, "/session/me") - .header(http::header::COOKIE, session_cookie_value) + .header(http::header::COOKIE, session_cookie_value.clone()) .expect_status(Some(StatusCode::OK)), ) .execute() @@ -1042,7 +1042,22 @@ async fn test_post_saml_response(cptestctx: &ControlPlaneTestContext) { .unwrap(); assert_eq!(session_me.display_name, "some@customer.com"); - assert_same_items(session_me.group_ids, silo_group_ids); + + let session_me: ResultsPage = NexusRequest::new( + RequestBuilder::new(client, Method::GET, "/session/me/groups") + .header(http::header::COOKIE, session_cookie_value) + .expect_status(Some(StatusCode::OK)), + ) + .execute() + .await + .expect("expected success") + .parsed_body() + .unwrap(); + + let session_me_group_ids = + session_me.items.iter().map(|g| g.id).collect::>(); + + assert_same_items(session_me_group_ids, silo_group_ids); } /// Order-agnostic vec equality diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index b3ebd936b0a..eb929544f19 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -15,6 +15,7 @@ device_auth_request /device/auth login_spoof /login logout /logout session_me /session/me +session_me_groups /session/me/groups API operations found with tag "images" OPERATION ID URL PATH diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index e5e472d696c..c160a12f3db 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -322,19 +322,6 @@ pub struct User { pub silo_id: Uuid, } -/// Client view of a [`User`] and their groups -#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)] -pub struct SessionMe { - pub id: Uuid, - /** Human-readable name that can identify the user */ - pub display_name: String, - - /** Uuid of the silo to which this user belongs */ - pub silo_id: Uuid, - - pub group_ids: Vec, -} - // SILO GROUPS /// Client view of a [`Group`] diff --git a/openapi/nexus.json b/openapi/nexus.json index 9363e9ad8e9..741894b5e70 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -5202,7 +5202,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/SessionMe" + "$ref": "#/components/schemas/User" } } } @@ -5216,6 +5216,66 @@ } } }, + "/session/me/groups": { + "get": { + "tags": [ + "hidden" + ], + "summary": "Fetch the silo groups the current user belongs to", + "operationId": "session_me_groups", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + }, + "style": "form" + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + }, + "style": "form" + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/IdSortMode" + }, + "style": "form" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GroupResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": true + } + }, "/session/me/sshkeys": { "get": { "tags": [ @@ -11002,38 +11062,6 @@ "technical_contact_email" ] }, - "SessionMe": { - "description": "Client view of a [`User`] and their groups", - "type": "object", - "properties": { - "display_name": { - "description": "Human-readable name that can identify the user", - "type": "string" - }, - "group_ids": { - "type": "array", - "items": { - "type": "string", - "format": "uuid" - } - }, - "id": { - "type": "string", - "format": "uuid" - }, - "silo_id": { - "description": "Uuid of the silo to which this user belongs", - "type": "string", - "format": "uuid" - } - }, - "required": [ - "display_name", - "group_ids", - "id", - "silo_id" - ] - }, "Silo": { "description": "Client view of a ['Silo']", "type": "object",