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.

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

initial implementation of local-only users #1784

Merged
merged 17 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions common/src/sql/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,11 @@ CREATE INDEX ON omicron.public.console_session (
time_created
);

-- This index is used to remove sessions for a user that's being deleted.
CREATE INDEX ON omicron.public.console_session (
silo_user_id
);

/*******************************************************************/

CREATE TYPE omicron.public.update_artifact_kind AS ENUM (
Expand Down Expand Up @@ -1532,6 +1537,11 @@ CREATE UNIQUE INDEX ON omicron.public.device_access_token (
client_id, device_code
);

-- This index is used to remove tokens for a user that's being deleted.
CREATE INDEX ON omicron.public.device_access_token (
silo_user_id
);

/*
* Roles built into the system
*
Expand Down
8 changes: 7 additions & 1 deletion nexus/db-model/src/silo_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub struct SiloUser {
#[diesel(embed)]
identity: SiloUserIdentity,

pub time_deleted: Option<chrono::DateTime<chrono::Utc>>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change deserves comment for future reference (but feel free to ignore this): this column was already part of the silo_user table and it's also in the definition of silo_user in schema.rs. It's not new in the database schema. One might think this field in this struct is already present via the identity field at L16 above, but that's only true for things that derive Resource. This derives Asset, which don't automatically get a time_deleted field. So we have to explicitly add it here.

The failure mode for not having this was that when I went to use check_if_exists()/execute_and_check(), I got this error:

[2022-10-07T01:42:14.308056515Z]  INFO: 4dcd44fe-92f8-41b7-9709-d57a03b4c148/dropshot_external/13170 on ivanova: request completed (req_id=c8658d5d-2708-4ab3-b4b2-1146851295d8, method=DELETE, remote_addr=127.0.0.1:56592, local_addr=127.0.0.1:38195, error_message_external="Internal Server Error", response_code=500)
    uri: /system/silos/local-only/identity-providers/local/users/id/ec8e5966-53ff-4866-86e8-d8db68783dfb
    --
    error_message_internal: Unknown diesel error accessing SiloUser ById(ec8e5966-53ff-4866-86e8-d8db68783dfb): Unexpected null for non-null column

The reason is that check_if_exists() generates SQL that selects all of the columns that Diesel knows about. Then execute_and_check() attempts to deserialize those columns into this type (SiloUser). I believe it does not use SiloUser::as_select() (as other things do), but by explicitly providing the Q generic argument to get_result_async(). I believe that causes the fields to be deserialized in whatever order they appear. There was a mismatch here because this field was missing. The particular error about a null value happened because the database was providing a null value for time_deleted, but it was being deserialized into a non-nullable field silo_id.

pub silo_id: Uuid,

/// The identity provider's ID for this user.
Expand All @@ -23,7 +24,12 @@ pub struct SiloUser {

impl SiloUser {
pub fn new(silo_id: Uuid, user_id: Uuid, external_id: String) -> Self {
Self { identity: SiloUserIdentity::new(user_id), silo_id, external_id }
Self {
identity: SiloUserIdentity::new(user_id),
time_deleted: None,
silo_id,
external_id,
}
}
}

Expand Down
16 changes: 11 additions & 5 deletions nexus/src/app/iam.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ impl super::Nexus {

// Silo users

pub async fn silo_users_list(
/// List users in the current Silo
pub async fn silo_users_list_current(
&self,
opctx: &OpContext,
pagparams: &DataPageParams<'_, Uuid>,
Expand All @@ -67,18 +68,23 @@ impl super::Nexus {
.authn
.silo_required()
.internal_context("listing current silo's users")?;
let authz_silo_user_list = authz::SiloUserList::new(authz_silo.clone());
self.db_datastore
.silo_users_list_by_id(opctx, &authz_silo, pagparams)
.silo_users_list_by_id(opctx, &authz_silo_user_list, pagparams)
.await
}

pub async fn silo_user_fetch_by_id(
/// Fetch the currently-authenticated Silo user
pub async fn silo_user_fetch_self(
&self,
opctx: &OpContext,
silo_user_id: &Uuid,
) -> LookupResult<db::model::SiloUser> {
let &actor = opctx
.authn
.actor_required()
.internal_context("loading current user")?;
let (.., db_silo_user) = LookupPath::new(opctx, &self.db_datastore)
.silo_user_id(*silo_user_id)
.silo_user_id(actor.actor_id())
.fetch()
.await?;
Ok(db_silo_user)
Expand Down
135 changes: 132 additions & 3 deletions nexus/src/app/silo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

//! Silos, Users, and SSH Keys.

use crate::authz::ApiResource;
use crate::context::OpContext;
use crate::db;
use crate::db::identity::{Asset, Resource};
Expand All @@ -14,13 +15,15 @@ use crate::external_api::params;
use crate::external_api::shared;
use crate::{authn, authz};
use anyhow::Context;
use omicron_common::api::external::CreateResult;
use nexus_db_model::UserProvisionType;
use omicron_common::api::external::DataPageParams;
use omicron_common::api::external::DeleteResult;
use omicron_common::api::external::Error;
use omicron_common::api::external::ListResultVec;
use omicron_common::api::external::LookupResult;
use omicron_common::api::external::UpdateResult;
use omicron_common::api::external::{CreateResult, ResourceType};
use std::str::FromStr;
use uuid::Uuid;

impl super::Nexus {
Expand Down Expand Up @@ -140,18 +143,138 @@ impl super::Nexus {

// Users

/// Helper function for looking up a user in a Silo
///
/// `LookupPath` lets you look up users directly, regardless of what Silo
/// they're in. This helper validates that they're in the expected Silo.
async fn silo_user_lookup_by_id(
&self,
opctx: &OpContext,
authz_silo: &authz::Silo,
silo_user_id: Uuid,
action: authz::Action,
) -> LookupResult<(authz::SiloUser, db::model::SiloUser)> {
let (_, authz_silo_user, db_silo_user) =
LookupPath::new(opctx, self.datastore())
.silo_user_id(silo_user_id)
.fetch_for(action)
.await?;
if db_silo_user.silo_id != authz_silo.id() {
return Err(authz_silo_user.not_found());
}

Ok((authz_silo_user, db_silo_user))
}

/// List the users in a Silo
pub async fn silo_list_users(
&self,
opctx: &OpContext,
silo_name: &Name,
pagparams: &DataPageParams<'_, Uuid>,
) -> ListResultVec<db::model::SiloUser> {
let (authz_silo,) = LookupPath::new(opctx, self.datastore())
.silo_name(silo_name)
.lookup_for(authz::Action::Read)
.await?;
let authz_silo_user_list = authz::SiloUserList::new(authz_silo);
self.db_datastore
.silo_users_list_by_id(opctx, &authz_silo_user_list, pagparams)
.await
}

/// Fetch a user in a Silo
pub async fn silo_user_fetch(
&self,
opctx: &OpContext,
silo_name: &Name,
silo_user_id: Uuid,
) -> LookupResult<db::model::SiloUser> {
let (.., db_silo_user) = LookupPath::new(opctx, &self.datastore())
.silo_user_id(silo_user_id)
let (authz_silo,) = LookupPath::new(opctx, self.datastore())
.silo_name(silo_name)
.lookup_for(authz::Action::Read)
.await?;
let (_, db_silo_user) = self
.silo_user_lookup_by_id(
opctx,
&authz_silo,
silo_user_id,
authz::Action::Read,
)
.await?;
Ok(db_silo_user)
}

// The "local" identity provider (available only in `LocalOnly` Silos)

/// Helper function for looking up a LocalOnly Silo by name
///
/// This is called from contexts that are trying to access the "local"
/// identity provider. On failure, it returns a 404 for that identity
/// provider.
async fn local_idp_fetch_silo(
&self,
opctx: &OpContext,
silo_name: &Name,
) -> LookupResult<(authz::Silo, db::model::Silo)> {
let (authz_silo, db_silo) = LookupPath::new(opctx, &self.db_datastore)
.silo_name(silo_name)
.fetch()
.await?;
if db_silo.user_provision_type != UserProvisionType::ApiOnly {
return Err(Error::not_found_by_name(
ResourceType::IdentityProvider,
&omicron_common::api::external::Name::from_str("local")
.unwrap(),
));
}
Ok((authz_silo, db_silo))
}

/// Create a user in a Silo's local identity provider
pub async fn local_idp_create_user(
&self,
opctx: &OpContext,
silo_name: &Name,
new_user_params: params::UserCreate,
) -> CreateResult<db::model::SiloUser> {
let (authz_silo, _) =
self.local_idp_fetch_silo(opctx, silo_name).await?;
let authz_silo_user_list = authz::SiloUserList::new(authz_silo.clone());
// TODO-cleanup This authz check belongs in silo_user_create().
opctx
.authorize(authz::Action::CreateChild, &authz_silo_user_list)
.await?;
let silo_user = db::model::SiloUser::new(
authz_silo.id(),
Uuid::new_v4(),
new_user_params.external_id.as_ref().to_owned(),
);
let (_, db_silo_user) =
self.datastore().silo_user_create(&authz_silo, silo_user).await?;
Ok(db_silo_user)
}

/// Delete a user in a Silo's local identity provider
pub async fn local_idp_delete_user(
&self,
opctx: &OpContext,
silo_name: &Name,
silo_user_id: Uuid,
) -> DeleteResult {
let (authz_silo, _) =
self.local_idp_fetch_silo(opctx, silo_name).await?;
let (authz_silo_user, _) = self
.silo_user_lookup_by_id(
opctx,
&authz_silo,
silo_user_id,
authz::Action::Delete,
)
.await?;
self.db_datastore.silo_user_delete(opctx, &authz_silo_user).await
}

/// Based on an authenticated subject, fetch or create a silo user
pub async fn silo_user_from_authenticated_subject(
&self,
Expand Down Expand Up @@ -378,6 +501,12 @@ impl super::Nexus {
.await?;
let authz_idp_list = authz::SiloIdentityProviderList::new(authz_silo);

if db_silo.user_provision_type != UserProvisionType::Jit {
return Err(Error::invalid_request(
"cannot create identity providers in this kind of Silo",
));
}

// This check is not strictly necessary yet. We'll check this
// permission in the DataStore when we actually update the list.
// But we check now to protect the code that fetches the descriptor from
Expand Down
57 changes: 57 additions & 0 deletions nexus/src/authz/api_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,63 @@ impl AuthorizedResource for SiloIdentityProviderList {
}
}

/// Synthetic resource describing the list of Users in a Silo
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct SiloUserList(Silo);

impl SiloUserList {
pub fn new(silo: Silo) -> SiloUserList {
SiloUserList(silo)
}

pub fn silo(&self) -> &Silo {
&self.0
}
}

impl oso::PolarClass for SiloUserList {
fn get_polar_class_builder() -> oso::ClassBuilder<Self> {
oso::Class::builder()
.with_equality_check()
.add_attribute_getter("silo", |list: &SiloUserList| list.0.clone())
}
}

impl AuthorizedResource for SiloUserList {
fn load_roles<'a, 'b, 'c, 'd, 'e, 'f>(
&'a self,
opctx: &'b OpContext,
datastore: &'c DataStore,
authn: &'d authn::Context,
roleset: &'e mut RoleSet,
) -> futures::future::BoxFuture<'f, Result<(), Error>>
where
'a: 'f,
'b: 'f,
'c: 'f,
'd: 'f,
'e: 'f,
{
// There are no roles on this resource, but we still need to load the
// Silo-related roles.
self.silo().load_roles(opctx, datastore, authn, roleset)
}

fn on_unauthorized(
&self,
_: &Authz,
error: Error,
_: AnyActor,
_: Action,
) -> Error {
error
}

fn polar_class(&self) -> oso::Class {
Self::get_polar_class()
}
}

// Main resource hierarchy: Organizations, Projects, and their resources

authz_resource! {
Expand Down
Loading