Serve console index at /settings/*, /device/verify, `/device/succ…

…ess` (#1324) * serve console index at /settings/* * use full helper for /device/verify too * also do /device/success * first pass at always including current URI in login_url * improve code slightly, add big comment
oxidecomputer · Jul 1, 2022 · 24958c5 · 24958c5
1 parent 15ecdef
commit 24958c5
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 57 deletions.
diff --git a/nexus/src/external_api/console_api.rs b/nexus/src/external_api/console_api.rs
@@ -496,27 +496,33 @@ pub struct LoginUrlQuery {
 /// Generate URL to IdP login form. Optional `state` param is included in query
 /// string if present, and will typically represent the URL to send the user
 /// back to after successful login.
-pub fn get_login_url(state: Option<String>) -> String {
-    // assume state is not URL encoded, so no risk of double encoding (dropshot
-    // decodes it on the way in)
+// TODO this does not know anything about IdPs, and it should. When the user is
+// logged out and hits an auth-gated route, if there are multiple IdPs and we
+// don't known which one they want to use, we need to send them to a page that
+// will allow them to choose among discoverable IdPs. However, there may be ways
+// to give ourselves a hint about which one they want, for example, by storing
+// that info in a browser cookie when they log in. When their session ends, we
+// will not be able to look at the dead session to find the silo or IdP (well,
+// maybe we can but we probably shouldn't) but we can look at the cookie and
+// default to sending them to the IdP indicated (though if they don't want that
+// one we need to make sure they can get to a different one). If there is no
+// cookie, we send them to the selector page. In any case, none of this is done
+// here yet. We go to /spoof_login no matter what.
+fn get_login_url(state: Option<String>) -> String {
+    // assume state is not already URL encoded
     let query = match state {
-        Some(state) if state.is_empty() => None,
-        Some(state) => Some(
+        Some(state) if !state.is_empty() => {
             serde_urlencoded::to_string(LoginUrlQuery { state: Some(state) })
-                // unwrap is safe because query.state was just deserialized out
-                // of a query param, so we know it's serializable
-                .unwrap(),
-        ),
-        None => None,
+                .ok() // in the strange event it's not serializable, no query
+        }
+        _ => None,
     };
     // Once we have IdP integration, this will be a URL for the IdP login page.
     // For now we point to our own placeholder login page.
-    let mut url = "/spoof_login".to_string();
-    if let Some(query) = query {
-        url.push('?');
-        url.push_str(query.as_str());
+    match query {
+        Some(query) => format!("/spoof_login?{query}"),
+        None => "/spoof_login".to_string(),
     }
-    url
 }
 
 /// Redirect to IdP login URL
@@ -558,6 +564,29 @@ pub async fn session_me(
     apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
 }
 
+pub async fn console_index_or_login_redirect(
+    rqctx: Arc<RequestContext<Arc<ServerContext>>>,
+) -> Result<Response<Body>, HttpError> {
+    let opctx = OpContext::for_external_api(&rqctx).await;
+
+    // if authed, serve console index.html with JS bundle in script tag
+    if let Ok(opctx) = opctx {
+        if opctx.authn.actor().is_some() {
+            return serve_console_index(rqctx.context()).await;
+        }
+    }
+
+    // otherwise redirect to idp
+
+    // put the current URI in the query string to redirect back to after login
+    let uri = rqctx.request.lock().await.uri().to_string();
+
+    Ok(Response::builder()
+        .status(StatusCode::FOUND)
+        .header(http::header::LOCATION, get_login_url(Some(uri)))
+        .body("".into())?)
+}
+
 // Dropshot does not have route match ranking and does not allow overlapping
 // route definitions, so we cannot have a catchall `/*` route for console pages
 // and then also define, e.g., `/api/blah/blah` and give the latter priority
@@ -575,28 +604,19 @@ pub async fn console_page(
     rqctx: Arc<RequestContext<Arc<ServerContext>>>,
     _path_params: Path<RestPathParam>,
 ) -> Result<Response<Body>, HttpError> {
-    let opctx = OpContext::for_external_api(&rqctx).await;
-
-    // if authed, serve HTML page with bundle in script tag
-
-    // HTML doesn't need to be static -- we'll probably find a reason to do some
-    // minimal templating, e.g., putting a CSRF token in the page
-
-    // amusingly, at least to start out, I don't think we care about the path
-    // because the real routing is all client-side. we serve the same HTML
-    // regardless, the app starts on the client and renders the right page and
-    // makes the right API requests.
-    if let Ok(opctx) = opctx {
-        if opctx.authn.actor().is_some() {
-            return serve_console_index(rqctx.context()).await;
-        }
-    }
+    console_index_or_login_redirect(rqctx).await
+}
 
-    // otherwise redirect to idp
-    Ok(Response::builder()
-        .status(StatusCode::FOUND)
-        .header(http::header::LOCATION, get_login_url(None))
-        .body("".into())?)
+#[endpoint {
+   method = GET,
+   path = "/settings/{path:.*}",
+   unpublished = true,
+}]
+pub async fn console_settings_page(
+    rqctx: Arc<RequestContext<Arc<ServerContext>>>,
+    _path_params: Path<RestPathParam>,
+) -> Result<Response<Body>, HttpError> {
+    console_index_or_login_redirect(rqctx).await
 }
 
 /// Fetch a static asset from `<static_dir>/assets`. 404 on virtually all

diff --git a/nexus/src/external_api/device_auth.rs b/nexus/src/external_api/device_auth.rs
@@ -9,7 +9,7 @@
 //! are for requesting access tokens that will be managed and used by
 //! the client to make other API requests.
 
-use super::console_api::{get_login_url, serve_console_index};
+use super::console_api::console_index_or_login_redirect;
 use super::views::{DeviceAccessTokenGrant, DeviceAuthResponse};
 use crate::context::OpContext;
 use crate::db::model::DeviceAccessToken;
@@ -21,7 +21,6 @@ use http::{header, Response, StatusCode};
 use hyper::Body;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use serde_urlencoded;
 use std::sync::Arc;
 use uuid::Uuid;
 
@@ -126,26 +125,20 @@ pub struct DeviceAuthVerify {
 }]
 pub async fn device_auth_verify(
     rqctx: Arc<RequestContext<Arc<ServerContext>>>,
-    params: Query<DeviceAuthVerify>,
+    _params: Query<DeviceAuthVerify>,
 ) -> Result<Response<Body>, HttpError> {
-    // If the user is authenticated, serve the console verification page.
-    if let Ok(opctx) = OpContext::for_external_api(&rqctx).await {
-        if opctx.authn.actor().is_some() {
-            return serve_console_index(rqctx.context()).await;
-        }
-    }
+    console_index_or_login_redirect(rqctx).await
+}
 
-    // Otherwise, redirect for authentication.
-    let params = params.into_inner();
-    let state_params = serde_urlencoded::to_string(serde_json::json!({
-        "user_code": params.user_code
-    }))
-    .map_err(|e| HttpError::for_internal_error(e.to_string()))?;
-    let state = Some(format!("/device/verify?{}", state_params));
-    Ok(Response::builder()
-        .status(StatusCode::FOUND)
-        .header(http::header::LOCATION, get_login_url(state))
-        .body("".into())?)
+#[endpoint {
+    method = GET,
+    path = "/device/success",
+    unpublished = true,
+}]
+pub async fn device_auth_success(
+    rqctx: Arc<RequestContext<Arc<ServerContext>>>,
+) -> Result<Response<Body>, HttpError> {
+    console_index_or_login_redirect(rqctx).await
 }
 
 /// Confirm an OAuth 2.0 Device Authorization Grant

diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs
@@ -228,6 +228,7 @@ pub fn external_api() -> NexusApiDescription {
 
         api.register(device_auth::device_auth_request)?;
         api.register(device_auth::device_auth_verify)?;
+        api.register(device_auth::device_auth_success)?;
         api.register(device_auth::device_auth_confirm)?;
         api.register(device_auth::device_access_token)?;
 

diff --git a/nexus/tests/integration_tests/console_api.rs b/nexus/tests/integration_tests/console_api.rs
@@ -116,7 +116,10 @@ async fn test_console_pages(cptestctx: &ControlPlaneTestContext) {
     // request to console page route without auth should redirect to IdP
     let _ = RequestBuilder::new(&testctx, Method::GET, "/orgs/irrelevant-path")
         .expect_status(Some(StatusCode::FOUND))
-        .expect_response_header(header::LOCATION, "/spoof_login")
+        .expect_response_header(
+            header::LOCATION,
+            "/spoof_login?state=%2Forgs%2Firrelevant-path",
+        )
         .execute()
         .await
         .expect("failed to redirect to IdP on auth failure");