-
Notifications
You must be signed in to change notification settings - Fork 45
[3/n] /v1/me/access-tokens
list and delete
#8227
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.
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
Changes from all commits
dc6ec38
938c077
a680b64
8dd613a
474d82f
9ace306
bbcc90d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,18 @@ use crate::authz; | |
use crate::context::OpContext; | ||
use crate::db::model::DeviceAccessToken; | ||
use crate::db::model::DeviceAuthRequest; | ||
use crate::db::pagination::paginated; | ||
use async_bb8_diesel::AsyncRunQueryDsl; | ||
use chrono::Utc; | ||
use diesel::prelude::*; | ||
use nexus_db_errors::ErrorHandler; | ||
use nexus_db_errors::public_error_from_diesel; | ||
use nexus_db_schema::schema::device_access_token; | ||
use omicron_common::api::external::CreateResult; | ||
use omicron_common::api::external::DataPageParams; | ||
use omicron_common::api::external::Error; | ||
use omicron_common::api::external::InternalContext; | ||
use omicron_common::api::external::ListResultVec; | ||
use omicron_common::api::external::LookupResult; | ||
use omicron_common::api::external::LookupType; | ||
use omicron_common::api::external::ResourceType; | ||
|
@@ -176,4 +181,64 @@ impl DataStore { | |
) | ||
}) | ||
} | ||
|
||
// Similar to session hard delete and silo group list, we do not do a | ||
// typical authz check, instead effectively encoding the policy here that | ||
// any user is allowed to list and delete their own tokens. When we add the | ||
// ability for silo admins to list and delete tokens from any user, we will | ||
// have to model these permissions properly in the polar policy. | ||
|
||
pub async fn current_user_token_list( | ||
&self, | ||
opctx: &OpContext, | ||
pagparams: &DataPageParams<'_, Uuid>, | ||
) -> ListResultVec<DeviceAccessToken> { | ||
let &actor = opctx | ||
.authn | ||
.actor_required() | ||
.internal_context("listing current user's tokens")?; | ||
|
||
use nexus_db_schema::schema::device_access_token::dsl; | ||
paginated(dsl::device_access_token, dsl::id, &pagparams) | ||
.filter(dsl::silo_user_id.eq(actor.actor_id())) | ||
// we don't have time_deleted on tokens. unfortunately this is not | ||
// indexed well. maybe it can be! | ||
.filter( | ||
dsl::time_expires | ||
.is_null() | ||
.or(dsl::time_expires.gt(Utc::now())), | ||
) | ||
.select(DeviceAccessToken::as_select()) | ||
.load_async(&*self.pool_connection_authorized(opctx).await?) | ||
.await | ||
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) | ||
} | ||
|
||
pub async fn current_user_token_delete( | ||
&self, | ||
opctx: &OpContext, | ||
token_id: Uuid, | ||
) -> Result<(), Error> { | ||
let &actor = opctx | ||
.authn | ||
.actor_required() | ||
.internal_context("deleting current user's token")?; | ||
|
||
use nexus_db_schema::schema::device_access_token::dsl; | ||
let num_deleted = diesel::delete(dsl::device_access_token) | ||
.filter(dsl::silo_user_id.eq(actor.actor_id())) | ||
.filter(dsl::id.eq(token_id)) | ||
.execute_async(&*self.pool_connection_authorized(opctx).await?) | ||
.await | ||
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; | ||
|
||
if num_deleted == 0 { | ||
return Err(Error::not_found_by_id( | ||
ResourceType::DeviceAccessToken, | ||
&token_id, | ||
)); | ||
} | ||
|
||
Ok(()) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a hard delete — might want to put that in the name, since it's unusual. We do that in the session delete method. If we wanted a soft delete, we could set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the hard delete is fine, setting |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -287,6 +287,11 @@ API operations found with tag "system/status" | |
OPERATION ID METHOD URL PATH | ||
ping GET /v1/ping | ||
|
||
API operations found with tag "tokens" | ||
OPERATION ID METHOD URL PATH | ||
current_user_access_token_delete DELETE /v1/me/access-tokens/{token_id} | ||
current_user_access_token_list GET /v1/me/access-tokens | ||
Comment on lines
+292
to
+293
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm in favor of putting this under |
||
|
||
API operations found with tag "vpcs" | ||
OPERATION ID METHOD URL PATH | ||
internet_gateway_create POST /v1/internet-gateways | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is important. We hack around the harder authz problem of modeling token properly by restricting these functions to the current actor (for now).