Skip to content

Commit

Permalink
session and authn interfaces should do authz checks (#912)
Browse files Browse the repository at this point in the history
  • Loading branch information
davepacheco authored Apr 14, 2022
1 parent 1982088 commit 190ec73
Show file tree
Hide file tree
Showing 20 changed files with 406 additions and 155 deletions.
1 change: 1 addition & 0 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ pub enum ResourceType {
Silo,
SiloUser,
ConsoleSession,
ConsoleSessionList,
Organization,
Project,
Dataset,
Expand Down
20 changes: 12 additions & 8 deletions nexus/src/authn/external/session_cookie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ pub const SESSION_COOKIE_SCHEME_NAME: authn::SchemeName =

/// Generate session cookie header
pub fn session_cookie_header_value(token: &str, max_age: Duration) -> String {
// TODO-security:(https://github.com/oxidecomputer/omicron/issues/249): We should
// insert "Secure;" back into this string.
// TODO-security:(https://github.com/oxidecomputer/omicron/issues/249): We
// should insert "Secure;" back into this string.
format!(
"{}={}; Path=/; HttpOnly; SameSite=Lax; Max-Age={}",
SESSION_COOKIE_COOKIE_NAME,
Expand Down Expand Up @@ -110,7 +110,8 @@ where
let actor =
Actor { id: session.silo_user_id(), silo_id: session.silo_id() };

// if the session has gone unused for longer than idle_timeout, it is expired
// if the session has gone unused for longer than idle_timeout, it is
// expired
let now = Utc::now();
if session.time_last_used() + ctx.session_idle_timeout() < now {
let expired_session = ctx.session_expire(token.clone()).await;
Expand All @@ -120,16 +121,18 @@ where
return SchemeResult::Failed(Reason::BadCredentials {
actor,
source: anyhow!(
"session expired due to idle timeout. last used: {}. time checked: {}. TTL: {}",
"session expired due to idle timeout. last used: {}. \
time checked: {}. TTL: {}",
session.time_last_used(),
now,
ctx.session_idle_timeout()
),
});
}

// if the user is still within the idle timeout, but the session has existed longer
// than absolute_timeout, it is expired and we can no longer extend the session
// if the user is still within the idle timeout, but the session has
// existed longer than absolute_timeout, it is expired and we can no
// longer extend the session
if session.time_created() + ctx.session_absolute_timeout() < now {
let expired_session = ctx.session_expire(token.clone()).await;
if expired_session.is_none() {
Expand All @@ -138,7 +141,8 @@ where
return SchemeResult::Failed(Reason::BadCredentials {
actor,
source: anyhow!(
"session expired due to absolute timeout. created: {}. last used: {}. time checked: {}. TTL: {}",
"session expired due to absolute timeout. created: {}. \
last used: {}. time checked: {}. TTL: {}",
session.time_created(),
session.time_last_used(),
now,
Expand All @@ -151,7 +155,7 @@ where
// authenticated for this request at this point. The next request might
// be wrongly considered idle, but that's a problem for the next
// request.
let updated_session = ctx.session_update_last_used(token.clone()).await;
let updated_session = ctx.session_update_last_used(token).await;
if updated_session.is_none() {
debug!(log, "failed to extend session")
}
Expand Down
15 changes: 15 additions & 0 deletions nexus/src/authn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub mod external;
pub mod saga;

pub use crate::db::fixed_data::user_builtin::USER_DB_INIT;
pub use crate::db::fixed_data::user_builtin::USER_EXTERNAL_AUTHN;
pub use crate::db::fixed_data::user_builtin::USER_INTERNAL_API;
pub use crate::db::fixed_data::user_builtin::USER_INTERNAL_READ;
pub use crate::db::fixed_data::user_builtin::USER_SAGA_RECOVERY;
Expand Down Expand Up @@ -120,6 +121,15 @@ impl Context {
)
}

/// Returns an authenticated context for use for authenticating external
/// requests
pub fn external_authn() -> Context {
Context::context_for_actor(
USER_EXTERNAL_AUTHN.id,
USER_EXTERNAL_AUTHN.silo_id,
)
}

/// Returns an authenticated context for Nexus-startup database
/// initialization
pub fn internal_db_init() -> Context {
Expand Down Expand Up @@ -159,6 +169,7 @@ mod test {
use super::USER_INTERNAL_READ;
use super::USER_SAGA_RECOVERY;
use super::USER_TEST_PRIVILEGED;
use crate::db::fixed_data::user_builtin::USER_EXTERNAL_AUTHN;

#[test]
fn test_internal_users() {
Expand All @@ -177,6 +188,10 @@ mod test {
let actor = authn.actor().unwrap();
assert_eq!(actor.id, USER_INTERNAL_READ.id);

let authn = Context::external_authn();
let actor = authn.actor().unwrap();
assert_eq!(actor.id, USER_EXTERNAL_AUTHN.id);

let authn = Context::internal_db_init();
let actor = authn.actor().unwrap();
assert_eq!(actor.id, USER_DB_INIT.id);
Expand Down
67 changes: 67 additions & 0 deletions nexus/src/authz/api_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,65 @@ impl AuthorizedResource for Fleet {
}
}

/// ConsoleSessionList is a synthetic resource used for modeling who has access
/// to create sessions.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct ConsoleSessionList;

pub const CONSOLE_SESSION_LIST: ConsoleSessionList = ConsoleSessionList {};

impl oso::PolarClass for ConsoleSessionList {
fn get_polar_class_builder() -> oso::ClassBuilder<Self> {
// Roles are not directly attached to ConsoleSessionList.
oso::Class::builder()
.with_equality_check()
.add_method(
"has_role",
|_: &ConsoleSessionList,
_actor: AuthenticatedActor,
_role: String| false,
)
.add_attribute_getter("fleet", |_| FLEET)
}
}

impl AuthorizedResource for ConsoleSessionList {
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,
{
load_roles_for_resource(
opctx,
datastore,
authn,
ResourceType::Fleet,
*FLEET_ID,
roleset,
)
.boxed()
}

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

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

authz_resource! {
Expand Down Expand Up @@ -267,6 +326,14 @@ authz_resource! {

// Miscellaneous resources nested directly below "Fleet"

authz_resource! {
name = "ConsoleSession",
parent = "Fleet",
primary_key = String,
roles_allowed = false,
polar_snippet = FleetChild,
}

authz_resource! {
name = "SiloUser",
parent = "Fleet",
Expand Down
22 changes: 15 additions & 7 deletions nexus/src/authz/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ impl Authz {
///
/// This function panics if we could not load the compiled-in Polar
/// configuration. That should be impossible outside of development.
pub fn new() -> Authz {
let oso = oso_generic::make_omicron_oso().expect("initializing Oso");
pub fn new(log: &slog::Logger) -> Authz {
let oso = oso_generic::make_omicron_oso(log).expect("initializing Oso");
Authz { oso }
}

Expand Down Expand Up @@ -179,16 +179,20 @@ mod test {
use std::sync::Arc;

fn authz_context_for_actor(
log: &slog::Logger,
authn: authn::Context,
datastore: Arc<DataStore>,
) -> Context {
let authz = Authz::new();
let authz = Authz::new(log);
Context::new(Arc::new(authn), Arc::new(authz), datastore)
}

fn authz_context_noauth(datastore: Arc<DataStore>) -> Context {
fn authz_context_noauth(
log: &slog::Logger,
datastore: Arc<DataStore>,
) -> Context {
let authn = authn::Context::internal_unauthenticated();
let authz = Authz::new();
let authz = Authz::new(log);
Context::new(Arc::new(authn), Arc::new(authz), datastore)
}

Expand All @@ -199,6 +203,7 @@ mod test {
let (opctx, datastore) =
crate::db::datastore::datastore_test(&logctx, &db).await;
let authz_privileged = authz_context_for_actor(
&logctx.log,
authn::Context::internal_test_user(),
Arc::clone(&datastore),
);
Expand All @@ -218,6 +223,7 @@ mod test {
omicron_common::api::external::Error::Forbidden
));
let authz_nobody = authz_context_for_actor(
&logctx.log,
authn::Context::test_context_for_actor(
authn::USER_TEST_UNPRIVILEGED.id,
),
Expand All @@ -227,7 +233,7 @@ mod test {
.authorize(&opctx, Action::Query, DATABASE)
.await
.expect("expected unprivileged user to be able to query database");
let authz_noauth = authz_context_noauth(datastore);
let authz_noauth = authz_context_noauth(&logctx.log, datastore);
authz_noauth
.authorize(&opctx, Action::Query, DATABASE)
.await
Expand All @@ -246,6 +252,7 @@ mod test {
crate::db::datastore::datastore_test(&logctx, &db).await;

let authz_privileged = authz_context_for_actor(
&logctx.log,
authn::Context::internal_test_user(),
Arc::clone(&datastore),
);
Expand All @@ -256,6 +263,7 @@ mod test {
"expected privileged user to be able to create organization",
);
let authz_nobody = authz_context_for_actor(
&logctx.log,
authn::Context::test_context_for_actor(
authn::USER_TEST_UNPRIVILEGED.id,
),
Expand All @@ -267,7 +275,7 @@ mod test {
.expect_err(
"expected unprivileged user not to be able to create organization",
);
let authz_noauth = authz_context_noauth(datastore);
let authz_noauth = authz_context_noauth(&logctx.log, datastore);
authz_noauth
.authorize(&opctx, Action::Query, DATABASE)
.await
Expand Down
30 changes: 29 additions & 1 deletion nexus/src/authz/omicron.polar
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ resource Fleet {
"create_child",
];

roles = [ "admin", "collaborator", "viewer" ];
roles = [
"admin",
"collaborator",
"viewer",

# internal roles
"external-authenticator"
];

# Fleet viewers can view Fleet-wide data
"list_children" if "viewer";
Expand Down Expand Up @@ -178,3 +185,24 @@ resource Project {
}
has_relation(organization: Organization, "parent_organization", project: Project)
if project.organization = organization;


# ConsoleSessionList is a synthetic resource used for modeling who has access
# to create sessions.
resource ConsoleSessionList {
permissions = [ "create_child" ];
relations = { parent_fleet: Fleet };
"create_child" if "external-authenticator" on "parent_fleet";
}
has_relation(fleet: Fleet, "parent_fleet", collection: ConsoleSessionList)
if collection.fleet = fleet;

# These rules grants the external authenticator role the permissions it needs to
# read silo users and modify their sessions. This is necessary for login to
# work.
has_permission(actor: AuthenticatedActor, "read", user: SiloUser)
if has_role(actor, "external-authenticator", user.fleet);
has_permission(actor: AuthenticatedActor, "read", session: ConsoleSession)
if has_role(actor, "external-authenticator", session.fleet);
has_permission(actor: AuthenticatedActor, "modify", session: ConsoleSession)
if has_role(actor, "external-authenticator", session.fleet);
7 changes: 6 additions & 1 deletion nexus/src/authz/oso_generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub(super) struct Init {

/// Returns an Oso handle suitable for authorizing using Omicron's authorization
/// rules
pub fn make_omicron_oso() -> Result<Oso, anyhow::Error> {
pub fn make_omicron_oso(log: &slog::Logger) -> Result<Oso, anyhow::Error> {
let mut oso = Oso::new();
let classes = [
// Hand-written classes
Expand All @@ -43,8 +43,10 @@ pub fn make_omicron_oso() -> Result<Oso, anyhow::Error> {
AuthenticatedActor::get_polar_class(),
Database::get_polar_class(),
Fleet::get_polar_class(),
ConsoleSessionList::get_polar_class(),
];
for c in classes {
trace!(log, "registering Oso class"; "class" => &c.name);
oso.register_class(c).context("registering class")?;
}

Expand All @@ -60,6 +62,7 @@ pub fn make_omicron_oso() -> Result<Oso, anyhow::Error> {
RouterRoute::init(),
VpcSubnet::init(),
// Fleet-level resources
ConsoleSession::init(),
Rack::init(),
RoleBuiltin::init(),
Silo::init(),
Expand All @@ -75,9 +78,11 @@ pub fn make_omicron_oso() -> Result<Oso, anyhow::Error> {
.join("\n");

for init in generated_inits {
trace!(log, "registering Oso class"; "class" => &init.polar_class.name);
oso.register_class(init.polar_class).context("registering class")?;
}

trace!(log, "full Oso configuration"; "config" => &polar_config);
oso.load_str(&polar_config).context("loading Polar (Oso) config")?;
Ok(oso)
}
Expand Down
Loading

0 comments on commit 190ec73

Please sign in to comment.