Skip to content

Commit 1b08ebd

Browse files
authored
[4/n] Let user request a specific TTL for a token (#8231)
Closes #8147. Built on #8137, #8214, and #8227. This is pretty straightforward, I think. The user gives us a TTL in seconds at token request time. We store it on the request row. When they come back in the later step to confirm the code and generate the token, we retrieve the TTL, validate that it is less than the silo max (if one is set), and we use it to generate the `time_expires` timestamp, which cannot be changed later. One slightly surprising bit is that we can't validate the TTL against the silo max at initial request time because we don't have any idea what silo the user is associated with until the confirm step. So probably want to make sure we are handling TTL validation errors nicely in the web console, because I think that's where they will show up.
1 parent a8aeac6 commit 1b08ebd

File tree

11 files changed

+243
-21
lines changed

11 files changed

+243
-21
lines changed

end-to-end-tests/src/bin/bootstrap.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ async fn main() -> Result<()> {
115115
deserialize_byte_stream(
116116
ctx.client
117117
.device_auth_request()
118-
.body(DeviceAuthRequest { client_id })
118+
.body(DeviceAuthRequest { client_id, ttl_seconds: None })
119119
.send()
120120
.await?,
121121
)

nexus/db-model/src/device_auth.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ use chrono::{DateTime, Duration, Utc};
1313
use nexus_types::external_api::views;
1414
use omicron_uuid_kinds::{AccessTokenKind, GenericUuid, TypedUuid};
1515
use rand::{Rng, RngCore, SeedableRng, distributions::Slice, rngs::StdRng};
16+
use std::num::NonZeroU32;
1617
use uuid::Uuid;
1718

19+
use crate::SqlU32;
1820
use crate::typed_uuid::DbTypedUuid;
1921

2022
/// Default timeout in seconds for client to authenticate for a token request.
@@ -32,6 +34,9 @@ pub struct DeviceAuthRequest {
3234
pub user_code: String,
3335
pub time_created: DateTime<Utc>,
3436
pub time_expires: DateTime<Utc>,
37+
38+
/// TTL requested by the user
39+
pub token_ttl_seconds: Option<SqlU32>,
3540
}
3641

3742
impl DeviceAuthRequest {
@@ -98,7 +103,10 @@ fn generate_user_code() -> String {
98103
}
99104

100105
impl DeviceAuthRequest {
101-
pub fn new(client_id: Uuid) -> Self {
106+
pub fn new(
107+
client_id: Uuid,
108+
requested_ttl_seconds: Option<NonZeroU32>,
109+
) -> Self {
102110
let now = Utc::now();
103111
Self {
104112
client_id,
@@ -107,6 +115,8 @@ impl DeviceAuthRequest {
107115
time_created: now,
108116
time_expires: now
109117
+ Duration::seconds(CLIENT_AUTHENTICATION_TIMEOUT),
118+
token_ttl_seconds: requested_ttl_seconds
119+
.map(|ttl| ttl.get().into()),
110120
}
111121
}
112122

nexus/db-model/src/schema_versions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
1616
///
1717
/// This must be updated when you change the database schema. Refer to
1818
/// schema/crdb/README.adoc in the root of this repository for details.
19-
pub const SCHEMA_VERSION: Version = Version::new(146, 0, 0);
19+
pub const SCHEMA_VERSION: Version = Version::new(147, 0, 0);
2020

2121
/// List of all past database schema versions, in *reverse* order
2222
///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
2828
// | leaving the first copy as an example for the next person.
2929
// v
3030
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
31+
KnownVersion::new(147, "device-auth-request-ttl"),
3132
KnownVersion::new(146, "silo-settings-token-expiration"),
3233
KnownVersion::new(145, "token-and-session-ids"),
3334
KnownVersion::new(144, "inventory-omicron-sled-config"),

nexus/db-schema/src/schema.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,7 @@ table! {
13601360
device_code -> Text,
13611361
time_created -> Timestamptz,
13621362
time_expires -> Timestamptz,
1363+
token_ttl_seconds -> Nullable<Int8>,
13631364
}
13641365
}
13651366

nexus/src/app/device_auth.rs

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ use nexus_db_queries::context::OpContext;
5353
use nexus_db_queries::db::model::{DeviceAccessToken, DeviceAuthRequest};
5454

5555
use anyhow::anyhow;
56-
use nexus_types::external_api::params::DeviceAccessTokenRequest;
56+
use nexus_types::external_api::params;
5757
use nexus_types::external_api::views;
5858
use omicron_common::api::external::{
5959
CreateResult, DataPageParams, Error, ListResultVec,
@@ -77,13 +77,17 @@ impl super::Nexus {
7777
pub(crate) async fn device_auth_request_create(
7878
&self,
7979
opctx: &OpContext,
80-
client_id: Uuid,
80+
params: params::DeviceAuthRequest,
8181
) -> CreateResult<DeviceAuthRequest> {
8282
// TODO-correctness: the `user_code` generated for a new request
8383
// is used as a primary key, but may potentially collide with an
8484
// existing outstanding request. So we should retry some (small)
8585
// number of times if inserting the new request fails.
86-
let auth_request = DeviceAuthRequest::new(client_id);
86+
87+
// Note that we cannot validate the TTL here against the silo max
88+
// because we do not know what silo we're talking about until verify
89+
let auth_request =
90+
DeviceAuthRequest::new(params.client_id, params.ttl_seconds);
8791
self.db_datastore.device_auth_request_create(opctx, auth_request).await
8892
}
8993

@@ -115,17 +119,30 @@ impl super::Nexus {
115119
.silo_auth_settings_view(opctx, &authz_silo)
116120
.await?;
117121

118-
// Create an access token record.
122+
let silo_max_ttl = silo_auth_settings.device_token_max_ttl_seconds;
123+
let requested_ttl = db_request.token_ttl_seconds;
124+
125+
// Validate the requested TTL against the silo's max TTL
126+
if let (Some(requested), Some(max)) = (requested_ttl, silo_max_ttl) {
127+
if requested > max.0.into() {
128+
return Err(Error::invalid_request(&format!(
129+
"Requested TTL {} seconds exceeds maximum \
130+
allowed TTL for this silo of {} seconds",
131+
requested, max
132+
)));
133+
}
134+
}
135+
136+
let time_expires = requested_ttl
137+
.or(silo_max_ttl)
138+
.map(|ttl| Utc::now() + Duration::seconds(ttl.0.into()));
139+
119140
let token = DeviceAccessToken::new(
120141
db_request.client_id,
121142
db_request.device_code,
122143
db_request.time_created,
123144
silo_user_id,
124-
// Token gets the max TTL for the silo (if there is one) until we
125-
// build a way for the user to ask for a different TTL
126-
silo_auth_settings
127-
.device_token_max_ttl_seconds
128-
.map(|ttl| Utc::now() + Duration::seconds(ttl.0.into())),
145+
time_expires,
129146
);
130147

131148
if db_request.time_expires < Utc::now() {
@@ -224,7 +241,7 @@ impl super::Nexus {
224241
pub(crate) async fn device_access_token(
225242
&self,
226243
opctx: &OpContext,
227-
params: DeviceAccessTokenRequest,
244+
params: params::DeviceAccessTokenRequest,
228245
) -> Result<Response<Body>, HttpError> {
229246
// RFC 8628 §3.4
230247
if params.grant_type != "urn:ietf:params:oauth:grant-type:device_code" {

nexus/src/external_api/http_entrypoints.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7856,9 +7856,8 @@ impl NexusExternalApi for NexusExternalApiImpl {
78567856
}
78577857
};
78587858

7859-
let model = nexus
7860-
.device_auth_request_create(&opctx, params.client_id)
7861-
.await?;
7859+
let model =
7860+
nexus.device_auth_request_create(&opctx, params).await?;
78627861
nexus.build_oauth_response(
78637862
StatusCode::OK,
78647863
&model.into_response(rqctx.server.using_tls(), host),

nexus/tests/integration_tests/device_auth.rs

Lines changed: 183 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
use std::num::NonZeroU32;
66

77
use chrono::Utc;
8-
use dropshot::ResultsPage;
98
use dropshot::test_util::ClientTestContext;
9+
use dropshot::{HttpErrorResponseBody, ResultsPage};
1010
use nexus_auth::authn::USER_TEST_UNPRIVILEGED;
1111
use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO;
1212
use nexus_db_queries::db::identity::{Asset, Resource};
@@ -55,7 +55,9 @@ async fn test_device_auth_flow(cptestctx: &ControlPlaneTestContext) {
5555
.expect("failed to reject device auth start without client_id");
5656

5757
let client_id = Uuid::new_v4();
58-
let authn_params = DeviceAuthRequest { client_id };
58+
// note that this exercises ttl_seconds being omitted from the body because
59+
// it's URL encoded, so None means it's omitted
60+
let authn_params = DeviceAuthRequest { client_id, ttl_seconds: None };
5961

6062
// Using a JSON encoded body fails.
6163
RequestBuilder::new(testctx, Method::POST, "/device/auth")
@@ -241,7 +243,7 @@ async fn test_device_auth_flow(cptestctx: &ControlPlaneTestContext) {
241243
/// as a string
242244
async fn get_device_token(testctx: &ClientTestContext) -> String {
243245
let client_id = Uuid::new_v4();
244-
let authn_params = DeviceAuthRequest { client_id };
246+
let authn_params = DeviceAuthRequest { client_id, ttl_seconds: None };
245247

246248
// Start a device authentication flow
247249
let auth_response: DeviceAuthResponse =
@@ -431,6 +433,184 @@ async fn test_device_token_expiration(cptestctx: &ControlPlaneTestContext) {
431433
assert_eq!(settings.device_token_max_ttl_seconds, None);
432434
}
433435

436+
// lets me stick whatever I want in this thing to be URL-encoded
437+
#[derive(serde::Serialize)]
438+
struct BadAuthReq {
439+
client_id: String,
440+
ttl_seconds: String,
441+
}
442+
443+
/// Test that 0 and negative values for ttl_seconds give immediate 400s
444+
#[nexus_test]
445+
async fn test_device_token_request_ttl_invalid(
446+
cptestctx: &ControlPlaneTestContext,
447+
) {
448+
let testctx = &cptestctx.external_client;
449+
450+
let auth_response = NexusRequest::new(
451+
RequestBuilder::new(testctx, Method::POST, "/device/auth")
452+
.allow_non_dropshot_errors()
453+
.body_urlencoded(Some(&BadAuthReq {
454+
client_id: Uuid::new_v4().to_string(),
455+
ttl_seconds: "0".to_string(),
456+
}))
457+
.expect_status(Some(StatusCode::BAD_REQUEST)),
458+
)
459+
.execute()
460+
// .execute_and_parse_unwrap::<DeviceAuthResponse>()
461+
.await
462+
.expect("expected an Ok(TestResponse)");
463+
464+
let error_body: serde_json::Value =
465+
serde_json::from_slice(&auth_response.body).unwrap();
466+
assert_eq!(
467+
error_body.get("message").unwrap().to_string(),
468+
"\"unable to parse URL-encoded body: ttl_seconds: \
469+
invalid value: integer `0`, expected a nonzero u32\""
470+
);
471+
472+
let auth_response = NexusRequest::new(
473+
RequestBuilder::new(testctx, Method::POST, "/device/auth")
474+
.allow_non_dropshot_errors()
475+
.body_urlencoded(Some(&BadAuthReq {
476+
client_id: Uuid::new_v4().to_string(),
477+
ttl_seconds: "-3".to_string(),
478+
}))
479+
.expect_status(Some(StatusCode::BAD_REQUEST)),
480+
)
481+
.execute()
482+
// .execute_and_parse_unwrap::<DeviceAuthResponse>()
483+
.await
484+
.expect("expected an Ok(TestResponse)");
485+
486+
let error_body: serde_json::Value =
487+
serde_json::from_slice(&auth_response.body).unwrap();
488+
assert_eq!(
489+
error_body.get("message").unwrap().to_string(),
490+
"\"unable to parse URL-encoded body: ttl_seconds: \
491+
invalid digit found in string\""
492+
);
493+
}
494+
495+
#[nexus_test]
496+
async fn test_device_token_request_ttl(cptestctx: &ControlPlaneTestContext) {
497+
let testctx = &cptestctx.external_client;
498+
499+
// Set silo max TTL to 10 seconds
500+
let settings = params::SiloAuthSettingsUpdate {
501+
device_token_max_ttl_seconds: NonZeroU32::new(10).into(),
502+
};
503+
let _: views::SiloAuthSettings =
504+
object_put(testctx, "/v1/auth-settings", &settings).await;
505+
506+
// Request TTL above the max should fail at verification time
507+
let invalid_ttl = DeviceAuthRequest {
508+
client_id: Uuid::new_v4(),
509+
ttl_seconds: NonZeroU32::new(20), // Above the 10 second max
510+
};
511+
512+
let auth_response = NexusRequest::new(
513+
RequestBuilder::new(testctx, Method::POST, "/device/auth")
514+
.body_urlencoded(Some(&invalid_ttl))
515+
.expect_status(Some(StatusCode::OK)),
516+
)
517+
.execute_and_parse_unwrap::<DeviceAuthResponse>()
518+
.await;
519+
520+
let confirm_params =
521+
DeviceAuthVerify { user_code: auth_response.user_code };
522+
523+
// Confirmation fails because requested TTL exceeds max
524+
let confirm_error = NexusRequest::new(
525+
RequestBuilder::new(testctx, Method::POST, "/device/confirm")
526+
.body(Some(&confirm_params))
527+
.expect_status(Some(StatusCode::BAD_REQUEST)),
528+
)
529+
.authn_as(AuthnMode::PrivilegedUser)
530+
.execute_and_parse_unwrap::<HttpErrorResponseBody>()
531+
.await;
532+
533+
// Check that the error message mentions TTL
534+
assert_eq!(confirm_error.error_code, Some("InvalidRequest".to_string()));
535+
assert_eq!(
536+
confirm_error.message,
537+
"Requested TTL 20 seconds exceeds maximum allowed TTL \
538+
for this silo of 10 seconds"
539+
);
540+
541+
// Request TTL below the max should succeed and be used
542+
let valid_ttl = DeviceAuthRequest {
543+
client_id: Uuid::new_v4(),
544+
ttl_seconds: NonZeroU32::new(3), // Below the 10 second max
545+
};
546+
547+
let auth_response = NexusRequest::new(
548+
RequestBuilder::new(testctx, Method::POST, "/device/auth")
549+
.body_urlencoded(Some(&valid_ttl))
550+
.expect_status(Some(StatusCode::OK)),
551+
)
552+
.execute_and_parse_unwrap::<DeviceAuthResponse>()
553+
.await;
554+
555+
let device_code = auth_response.device_code;
556+
let user_code = auth_response.user_code;
557+
let confirm_params = DeviceAuthVerify { user_code };
558+
559+
// this time will be pretty close to the now() used on the server when
560+
// calculating expiration time
561+
let t0 = Utc::now();
562+
563+
// Confirmation should succeed
564+
NexusRequest::new(
565+
RequestBuilder::new(testctx, Method::POST, "/device/confirm")
566+
.body(Some(&confirm_params))
567+
.expect_status(Some(StatusCode::NO_CONTENT)),
568+
)
569+
.authn_as(AuthnMode::PrivilegedUser)
570+
.execute()
571+
.await
572+
.expect("failed to confirm");
573+
574+
let token_params = DeviceAccessTokenRequest {
575+
grant_type: "urn:ietf:params:oauth:grant-type:device_code".to_string(),
576+
device_code,
577+
client_id: valid_ttl.client_id,
578+
};
579+
580+
// Get the token
581+
let token_grant = NexusRequest::new(
582+
RequestBuilder::new(testctx, Method::POST, "/device/token")
583+
.allow_non_dropshot_errors()
584+
.body_urlencoded(Some(&token_params))
585+
.expect_status(Some(StatusCode::OK)),
586+
)
587+
.authn_as(AuthnMode::PrivilegedUser)
588+
.execute_and_parse_unwrap::<DeviceAccessTokenGrant>()
589+
.await;
590+
591+
// Verify the token has roughly the correct expiration time. One second
592+
// threshold is sufficient to confirm it's not getting the silo max of 10
593+
// seconds. Locally, I saw diffs as low as 14ms.
594+
let tokens = get_tokens_priv(testctx).await;
595+
let time_expires = tokens[0].time_expires.unwrap();
596+
let expected_expires = t0 + Duration::from_secs(3);
597+
let diff_ms = (time_expires - expected_expires).num_milliseconds().abs();
598+
assert!(diff_ms <= 1000, "time diff was {diff_ms} ms. should be near zero");
599+
600+
// Token should work initially
601+
project_list(&testctx, &token_grant.access_token, StatusCode::OK)
602+
.await
603+
.expect("token should work initially");
604+
605+
// Wait for token to expire
606+
sleep(Duration::from_secs(4)).await;
607+
608+
// Token is expired
609+
project_list(&testctx, &token_grant.access_token, StatusCode::UNAUTHORIZED)
610+
.await
611+
.expect("token should be expired");
612+
}
613+
434614
async fn get_tokens_priv(
435615
testctx: &ClientTestContext,
436616
) -> Vec<views::DeviceAccessToken> {

nexus/types/src/external_api/params.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2467,6 +2467,9 @@ impl TryFrom<String> for RelativeUri {
24672467
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
24682468
pub struct DeviceAuthRequest {
24692469
pub client_id: Uuid,
2470+
/// Optional lifetime for the access token in seconds. If not specified, the
2471+
/// silo's max TTL will be used (if set).
2472+
pub ttl_seconds: Option<NonZeroU32>,
24702473
}
24712474

24722475
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]

0 commit comments

Comments
 (0)