Skip to content

Commit 773b7b8

Browse files
david-crespodavid-crespo
authored
Refactor login endpoints, instrument console endpoints (#7374)
Pulling these refactors out of #7339 because they're mechanical and just add noise. The point is to make it a cleaner diff when we add the function calls or wrapper code that creates audit log entries, as well as to clean up the `device_auth` (eliminated) and `console_api` (shrunken substantially) files, which have always been a little out of place. ### Refactors With the change to a trait-based Dropshot API, the already weird `console_api` and `device_auth` modules became even weirder, because the actual endpoint definitions were moved out of those files and into `http_entrypoints.rs`, but they still called functions that lived in the other files. These functions were redundant and had signatures more or less identical to the endpoint handlers. That's the main reason we lose 90 lines here. Before we had ``` http_entrypoints.rs -> console_api/device_auth -> nexus/src/app functions ``` Now we (mostly) cut out the middleman: ``` http_entrypoints.rs -> nexus/src/app functions ``` Some of what was in the middle moved up into the endpoint handlers, some moved "down" into the nexus "service layer" functions. ### One (1) functional change The one functional change is that the console endpoints are all instrumented now.
1 parent 1f0c185 commit 773b7b8

File tree

11 files changed

+554
-639
lines changed

11 files changed

+554
-639
lines changed

‎nexus/external-api/src/lib.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3033,6 +3033,11 @@ pub trait NexusExternalApi {
30333033
) -> Result<HttpResponseFound, HttpError>;
30343034

30353035
// Console API: Pages
3036+
//
3037+
// Dropshot does not have route match ranking and does not allow overlapping
3038+
// route definitions, so we cannot use a catchall `/*` route for console pages
3039+
// because it would overlap with the API routes definitions. So instead we have
3040+
// to manually define more specific routes.
30363041

30373042
#[endpoint {
30383043
method = GET,

‎nexus/src/app/device_auth.rs‎

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,30 @@
4444
//! In the current implementation, we use long-lived random tokens,
4545
//! but that may change in the future.
4646
47-
use crate::external_api::device_auth::DeviceAccessTokenResponse;
47+
use dropshot::{Body, HttpError};
48+
use http::{header, Response, StatusCode};
4849
use nexus_db_queries::authn::{Actor, Reason};
4950
use nexus_db_queries::authz;
5051
use nexus_db_queries::context::OpContext;
5152
use nexus_db_queries::db::lookup::LookupPath;
5253
use nexus_db_queries::db::model::{DeviceAccessToken, DeviceAuthRequest};
5354

55+
use nexus_types::external_api::params::DeviceAccessTokenRequest;
56+
use nexus_types::external_api::views;
5457
use omicron_common::api::external::{CreateResult, Error};
5558

5659
use chrono::Utc;
60+
use serde::Serialize;
5761
use uuid::Uuid;
5862

63+
#[derive(Debug)]
64+
pub enum DeviceAccessTokenResponse {
65+
Granted(DeviceAccessToken),
66+
Pending,
67+
#[allow(dead_code)]
68+
Denied,
69+
}
70+
5971
impl super::Nexus {
6072
/// Start a device authorization grant flow.
6173
/// Corresponds to steps 1 & 2 in the flow description above.
@@ -88,7 +100,7 @@ impl super::Nexus {
88100
.fetch()
89101
.await?;
90102

91-
let (.., authz_user) = LookupPath::new(opctx, &self.datastore())
103+
let (.., authz_user) = LookupPath::new(opctx, &self.db_datastore)
92104
.silo_user_id(silo_user_id)
93105
.lookup_for(authz::Action::CreateChild)
94106
.await?;
@@ -180,4 +192,77 @@ impl super::Nexus {
180192

181193
Ok(Actor::SiloUser { silo_user_id, silo_id })
182194
}
195+
196+
pub(crate) async fn device_access_token(
197+
&self,
198+
opctx: &OpContext,
199+
params: DeviceAccessTokenRequest,
200+
) -> Result<Response<Body>, HttpError> {
201+
// RFC 8628 §3.4
202+
if params.grant_type != "urn:ietf:params:oauth:grant-type:device_code" {
203+
return self.build_oauth_response(
204+
StatusCode::BAD_REQUEST,
205+
&serde_json::json!({
206+
"error": "unsupported_grant_type"
207+
}),
208+
);
209+
}
210+
211+
// RFC 8628 §3.5
212+
use DeviceAccessTokenResponse::*;
213+
match self
214+
.device_access_token_fetch(
215+
&opctx,
216+
params.client_id,
217+
params.device_code,
218+
)
219+
.await
220+
{
221+
Ok(response) => match response {
222+
Granted(token) => self.build_oauth_response(
223+
StatusCode::OK,
224+
&views::DeviceAccessTokenGrant::from(token),
225+
),
226+
Pending => self.build_oauth_response(
227+
StatusCode::BAD_REQUEST,
228+
&serde_json::json!({
229+
"error": "authorization_pending"
230+
}),
231+
),
232+
Denied => self.build_oauth_response(
233+
StatusCode::BAD_REQUEST,
234+
&serde_json::json!({
235+
"error": "access_denied"
236+
}),
237+
),
238+
},
239+
Err(error) => self.build_oauth_response(
240+
StatusCode::BAD_REQUEST,
241+
&serde_json::json!({
242+
"error": "invalid_request",
243+
"error_description": format!("{}", error),
244+
}),
245+
),
246+
}
247+
}
248+
249+
/// OAuth 2.0 error responses use 400 (Bad Request) with specific `error`
250+
/// parameter values to indicate protocol errors (see RFC 6749 §5.2).
251+
/// This is different from Dropshot's error `message` parameter, so we
252+
/// need a custom response builder.
253+
pub(crate) fn build_oauth_response<T>(
254+
&self,
255+
status: StatusCode,
256+
body: &T,
257+
) -> Result<Response<Body>, HttpError>
258+
where
259+
T: ?Sized + Serialize,
260+
{
261+
let body = serde_json::to_string(body)
262+
.map_err(|e| HttpError::for_internal_error(e.to_string()))?;
263+
Ok(Response::builder()
264+
.status(status)
265+
.header(header::CONTENT_TYPE, "application/json")
266+
.body(body.into())?)
267+
}
183268
}

‎nexus/src/app/login.rs‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
4+
5+
use dropshot::{http_response_found, HttpError, HttpResponseFound};
6+
use nexus_auth::context::OpContext;
7+
use nexus_db_model::{ConsoleSession, Name};
8+
use nexus_db_queries::authn::silos::IdentityProviderType;
9+
use nexus_db_queries::db::identity::Asset;
10+
use nexus_types::external_api::{params::RelativeUri, shared::RelayState};
11+
use omicron_common::api::external::Error;
12+
13+
impl super::Nexus {
14+
pub(crate) async fn login_saml_redirect(
15+
&self,
16+
opctx: &OpContext,
17+
silo_name: &Name,
18+
provider_name: &Name,
19+
redirect_uri: Option<RelativeUri>,
20+
) -> Result<HttpResponseFound, HttpError> {
21+
let (.., identity_provider) = self
22+
.datastore()
23+
.identity_provider_lookup(&opctx, silo_name, provider_name)
24+
.await?;
25+
26+
match identity_provider {
27+
IdentityProviderType::Saml(saml_identity_provider) => {
28+
// Relay state is sent to the IDP, to be sent back to the SP
29+
// after a successful login.
30+
let relay_state =
31+
RelayState { redirect_uri }.to_encoded().map_err(|e| {
32+
HttpError::for_internal_error(format!(
33+
"encoding relay state failed: {}",
34+
e
35+
))
36+
})?;
37+
38+
let sign_in_url = saml_identity_provider
39+
.sign_in_url(Some(relay_state))
40+
.map_err(|e| {
41+
HttpError::for_internal_error(e.to_string())
42+
})?;
43+
44+
http_response_found(sign_in_url)
45+
}
46+
}
47+
}
48+
49+
pub(crate) async fn login_saml(
50+
&self,
51+
opctx: &OpContext,
52+
body_bytes: dropshot::UntypedBody,
53+
silo_name: &Name,
54+
provider_name: &Name,
55+
) -> Result<(ConsoleSession, String), HttpError> {
56+
let (authz_silo, db_silo, identity_provider) = self
57+
.datastore()
58+
.identity_provider_lookup(&opctx, silo_name, provider_name)
59+
.await?;
60+
let (authenticated_subject, relay_state_string) =
61+
match identity_provider {
62+
IdentityProviderType::Saml(saml_identity_provider) => {
63+
let body_bytes = dbg!(body_bytes.as_str())?;
64+
saml_identity_provider.authenticated_subject(
65+
&body_bytes,
66+
self.samael_max_issue_delay(),
67+
)?
68+
}
69+
};
70+
let relay_state =
71+
relay_state_string.and_then(|v| RelayState::from_encoded(v).ok());
72+
let user = self
73+
.silo_user_from_authenticated_subject(
74+
&opctx,
75+
&authz_silo,
76+
&db_silo,
77+
&authenticated_subject,
78+
)
79+
.await?;
80+
let session = self.create_session(opctx, user).await?;
81+
let next_url = relay_state
82+
.and_then(|r| r.redirect_uri)
83+
.map(|u| u.to_string())
84+
.unwrap_or_else(|| "/".to_string());
85+
Ok((session, next_url))
86+
}
87+
88+
// TODO: move this logic, it's weird
89+
pub(crate) async fn create_session(
90+
&self,
91+
opctx: &OpContext,
92+
user: Option<nexus_db_queries::db::model::SiloUser>,
93+
) -> Result<ConsoleSession, Error> {
94+
let session = match user {
95+
Some(user) => self.session_create(&opctx, user.id()).await?,
96+
None => Err(Error::Unauthenticated {
97+
internal_message: String::from(
98+
"no matching user found or credentials were not valid",
99+
),
100+
})?,
101+
};
102+
Ok(session)
103+
}
104+
}

‎nexus/src/app/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ mod instance;
6363
mod instance_network;
6464
mod internet_gateway;
6565
mod ip_pool;
66+
mod login;
6667
mod metrics;
6768
mod network_interface;
6869
pub(crate) mod oximeter;

‎nexus/src/app/session.rs‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,26 +46,12 @@ impl super::Nexus {
4646
.await;
4747

4848
match fetch_result {
49-
Err(e) => {
50-
match e {
51-
Error::ObjectNotFound { type_name: _, lookup_type: _ } => {
52-
// if the silo user was deleted, they're not allowed to
53-
// log in :)
54-
return Ok(false);
55-
}
56-
57-
_ => {
58-
return Err(e);
59-
}
60-
}
61-
}
62-
63-
Ok(_) => {
64-
// they're allowed
65-
}
49+
// if the silo user was deleted, they're not allowed to log in :)
50+
Err(Error::ObjectNotFound { .. }) => Ok(false),
51+
Err(e) => Err(e),
52+
// they're allowed
53+
Ok(_) => Ok(true),
6654
}
67-
68-
Ok(true)
6955
}
7056

7157
pub(crate) async fn session_create(

0 commit comments

Comments
 (0)