Skip to content

Commit f83ca6f

Browse files
authored
Cleanup following device token fix #9411 (#9414)
We wanted to merge #9411 quickly but there was some test cleanup to do. Claude did a pretty amazing job with the vaguest prompt: > I want to factor out `test_device_token_cannot_extend_expiration` so it's more readable
1 parent 2a7b3ec commit f83ca6f

File tree

7 files changed

+223
-229
lines changed

7 files changed

+223
-229
lines changed

nexus/auth/src/authn/external/session_cookie.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -396,13 +396,7 @@ mod test {
396396
}]),
397397
};
398398
let result = authn_with_cookie(&context, Some("session=abc")).await;
399-
assert!(matches!(
400-
result,
401-
SchemeResult::Authenticated(Details {
402-
actor: _,
403-
device_token_expiration: _
404-
})
405-
));
399+
assert!(matches!(result, SchemeResult::Authenticated(Details { .. })));
406400

407401
// valid cookie should have updated time_last_used
408402
let sessions = context.sessions.lock().unwrap();

nexus/src/app/device_auth.rs

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -142,32 +142,32 @@ impl super::Nexus {
142142
// token is being used)
143143

144144
// Validate the requested TTL against the silo's max TTL
145-
if let Some(max) = silo_max_ttl {
146-
if requested_ttl > max.0.into() {
147-
return Err(Error::invalid_request(&format!(
148-
"Requested TTL {} seconds exceeds maximum allowed \
149-
TTL for this silo of {} seconds",
150-
requested_ttl, max
151-
)));
152-
}
145+
if let Some(max) = silo_max_ttl
146+
&& requested_ttl > max.0.into()
147+
{
148+
return Err(Error::invalid_request(&format!(
149+
"Requested TTL {} seconds exceeds maximum allowed \
150+
TTL for this silo of {} seconds",
151+
requested_ttl, max
152+
)));
153153
};
154154

155155
let requested_exp =
156156
Utc::now() + Duration::seconds(requested_ttl.0.into());
157157

158158
// If currently authenticated via token, error if requested exceeds it
159-
if let Some(auth_exp) = opctx.authn.device_token_expiration() {
160-
if requested_exp > auth_exp {
161-
return Err(Error::invalid_request(
162-
"Requested token TTL would exceed the expiration time \
163-
of the token being used to authenticate the confirm \
164-
request. To get the full requested TTL, confirm \
165-
this token using a web console session. Alternatively, \
166-
omit requested TTL to get a token with the longest \
167-
allowed lifetime, determined by the lesser of the silo \
168-
max and the current token's expiration time.",
169-
));
170-
}
159+
if let Some(auth_exp) = opctx.authn.device_token_expiration()
160+
&& requested_exp > auth_exp
161+
{
162+
return Err(Error::invalid_request(
163+
"Requested token TTL would exceed the expiration time \
164+
of the token being used to authenticate the confirm \
165+
request. To get the full requested TTL, confirm \
166+
this token using a web console session. Alternatively, \
167+
omit requested TTL to get a token with the longest \
168+
allowed lifetime, determined by the lesser of the silo \
169+
max and the current token's expiration time.",
170+
));
171171
}
172172

173173
Some(requested_exp)

nexus/test-utils/src/http_testing.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,7 @@ pub enum AuthnMode {
549549
PrivilegedUser,
550550
SiloUser(SiloUserUuid),
551551
Session(String),
552+
DeviceToken(String),
552553
}
553554

554555
impl AuthnMode {
@@ -579,6 +580,10 @@ impl AuthnMode {
579580
let header_value = format!("session={}", session_token);
580581
parse_header_pair(http::header::COOKIE, header_value)
581582
}
583+
AuthnMode::DeviceToken(token) => {
584+
let header_value = format!("Bearer {}", token);
585+
parse_header_pair(http::header::AUTHORIZATION, header_value)
586+
}
582587
}
583588
}
584589
}

nexus/test-utils/src/resource_helpers.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,20 +1316,17 @@ pub async fn create_session_for_user(
13161316
.to_string()
13171317
}
13181318

1319-
/// Log in with test suite password, return session cookie (formatted for Cookie
1320-
/// header)
1319+
/// Log in with test suite password. Returns session token.
13211320
pub async fn create_console_session<N: NexusServer>(
13221321
cptestctx: &ControlPlaneTestContext<N>,
13231322
) -> String {
1324-
let token = create_session_for_user(
1323+
create_session_for_user(
13251324
&cptestctx.external_client,
13261325
cptestctx.silo_name.as_str(),
13271326
cptestctx.user_name.as_ref(),
13281327
TEST_SUITE_PASSWORD,
13291328
)
1330-
.await;
1331-
1332-
format!("session={}", token)
1329+
.await
13331330
}
13341331

13351332
#[derive(Debug)]

nexus/tests/integration_tests/audit_log.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ async fn test_audit_log_list(ctx: &ControlPlaneTestContext) {
6161
assert_eq!(audit_log.items.len(), 1);
6262

6363
// this this creates its own entry
64-
let session_cookie = create_console_session(ctx).await;
64+
let session_cookie =
65+
format!("session={}", create_console_session(ctx).await);
6566

6667
let t3 = Utc::now(); // after second entry
6768

nexus/tests/integration_tests/console_api.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ async fn test_sessions(cptestctx: &ControlPlaneTestContext) {
5252
.expect("failed to clear cookie and 204 on logout");
5353

5454
// log in and pull the token out of the header so we can use it for authed requests
55-
let session_token = create_console_session(cptestctx).await;
55+
let session_cookie =
56+
format!("session={}", create_console_session(cptestctx).await);
5657

5758
let project_params = ProjectCreate {
5859
identity: IdentityMetadataCreateParams {
@@ -101,7 +102,7 @@ async fn test_sessions(cptestctx: &ControlPlaneTestContext) {
101102

102103
// now make same requests with cookie
103104
RequestBuilder::new(&testctx, Method::POST, "/v1/projects")
104-
.header(header::COOKIE, &session_token)
105+
.header(header::COOKIE, &session_cookie)
105106
.body(Some(&project_params))
106107
// TODO: explicit expect_status not needed. decide whether to keep it anyway
107108
.expect_status(Some(StatusCode::CREATED))
@@ -110,7 +111,7 @@ async fn test_sessions(cptestctx: &ControlPlaneTestContext) {
110111
.expect("failed to create org with session cookie");
111112

112113
RequestBuilder::new(&testctx, Method::GET, "/projects/whatever")
113-
.header(header::COOKIE, &session_token)
114+
.header(header::COOKIE, &session_cookie)
114115
.expect_console_asset()
115116
.execute()
116117
.await
@@ -124,7 +125,7 @@ async fn test_sessions(cptestctx: &ControlPlaneTestContext) {
124125

125126
// logout with an actual session should delete the session in the db
126127
RequestBuilder::new(&testctx, Method::POST, "/v1/logout")
127-
.header(header::COOKIE, &session_token)
128+
.header(header::COOKIE, &session_cookie)
128129
.expect_status(Some(StatusCode::NO_CONTENT))
129130
// logout also clears the cookie client-side
130131
.expect_response_header(
@@ -151,15 +152,15 @@ async fn test_sessions(cptestctx: &ControlPlaneTestContext) {
151152
// now the same requests with the same session cookie should 401/302 because
152153
// logout also deletes the session server-side
153154
RequestBuilder::new(&testctx, Method::POST, "/v1/projects")
154-
.header(header::COOKIE, &session_token)
155+
.header(header::COOKIE, &session_cookie)
155156
.body(Some(&project_params))
156157
.expect_status(Some(StatusCode::UNAUTHORIZED))
157158
.execute()
158159
.await
159160
.expect("failed to get 401 for unauthed API request");
160161

161162
RequestBuilder::new(&testctx, Method::GET, "/projects/whatever")
162-
.header(header::COOKIE, &session_token)
163+
.header(header::COOKIE, &session_cookie)
163164
.expect_status(Some(StatusCode::FOUND))
164165
.execute()
165166
.await
@@ -173,8 +174,9 @@ async fn expect_console_page(
173174
) {
174175
let mut builder = RequestBuilder::new(testctx, Method::GET, path);
175176

176-
if let Some(session_token) = session_token {
177-
builder = builder.header(http::header::COOKIE, &session_token)
177+
if let Some(token) = session_token {
178+
builder =
179+
builder.header(http::header::COOKIE, &format!("session={token}"))
178180
}
179181

180182
let console_page = builder
@@ -954,13 +956,13 @@ async fn test_session_idle_timeout_deletes_session() {
954956
let testctx = &cptestctx.external_client;
955957

956958
// Start session
957-
let session_cookie = create_console_session(&cptestctx).await;
959+
let session_token = create_console_session(&cptestctx).await;
958960

959961
// sleep here not necessary given TTL of 0
960962

961963
// Make a request with the expired session cookie
962964
let me_response = RequestBuilder::new(testctx, Method::GET, "/v1/me")
963-
.header(header::COOKIE, &session_cookie)
965+
.header(header::COOKIE, &format!("session={}", session_token))
964966
.expect_status(Some(StatusCode::UNAUTHORIZED))
965967
.execute()
966968
.await
@@ -977,10 +979,9 @@ async fn test_session_idle_timeout_deletes_session() {
977979
let opctx =
978980
OpContext::for_tests(cptestctx.logctx.log.new(o!()), datastore.clone());
979981

980-
let token = session_cookie.strip_prefix("session=").unwrap();
981982
let db_token_error = nexus
982983
.datastore()
983-
.session_lookup_by_token(&opctx, token.to_string())
984+
.session_lookup_by_token(&opctx, session_token)
984985
.await
985986
.expect_err("session should be deleted");
986987
assert_matches::assert_matches!(

0 commit comments

Comments
 (0)