Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serve console index at /settings/*, /device/verify, /device/success #1324

Merged
merged 5 commits into from
Jul 1, 2022
Merged

Serve console index at /settings/*, /device/verify, /device/success #1324

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d3b9e36
serve console index at /settings/*
david-crespo Jun 30, 2022
6db573c
use full helper for /device/verify too
david-crespo Jun 30, 2022
92b870a
also do /device/success
david-crespo Jun 30, 2022
5c33d76
first pass at always including current URI in login_url
david-crespo Jun 30, 2022
1ef808a
improve code slightly, add big comment
david-crespo Jun 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 33 additions & 15 deletions nexus/src/external_api/console_api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,22 +558,8 @@ pub async fn session_me(
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
}

// 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
// because it's a more specific match. So for now we simply give the console
// catchall route a prefix to avoid overlap. Long-term, if a route prefix is
// part of the solution, we would probably prefer it to be on the API endpoints,
// not on the console pages. Conveniently, all the console page routes start
// with /orgs already.
#[endpoint {
method = GET,
path = "/orgs/{path:.*}",
unpublished = true,
}]
pub async fn console_page(
pub async fn console_index_or_login_redirect(
rqctx: Arc<RequestContext<Arc<ServerContext>>>,
_path_params: Path<RestPathParam>,
) -> Result<Response<Body>, HttpError> {
let opctx = OpContext::for_external_api(&rqctx).await;

Expand All @@ -599,6 +585,38 @@ pub async fn console_page(
.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
// because it's a more specific match. So for now we simply give the console
// catchall route a prefix to avoid overlap. Long-term, if a route prefix is
// part of the solution, we would probably prefer it to be on the API endpoints,
// not on the console pages. Conveniently, all the console page routes start
// with /orgs already.
#[endpoint {
method = GET,
path = "/orgs/{path:.*}",
unpublished = true,
}]
pub async fn console_page(
rqctx: Arc<RequestContext<Arc<ServerContext>>>,
_path_params: Path<RestPathParam>,
) -> Result<Response<Body>, HttpError> {
console_index_or_login_redirect(rqctx).await
}

#[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
/// errors. No auth. NO SENSITIVE FILES.
#[endpoint {
Expand Down
33 changes: 13 additions & 20 deletions nexus/src/external_api/device_auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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())?)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plotnick I just noticed my version does not automatically put the current path + query in state like yours does (I thought it did). I will fix that

#[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
Expand Down
1 change: 1 addition & 0 deletions nexus/src/external_api/http_entrypoints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand Down