Refactor device auth schema (#1344)

Device auth request records are now short-lived so that we
can use `user_code` as a primary key.

Datastore authz is implemented for all but access token lookup,
which needs to be looked up both by `token` (primary key)
and the unique combination of `client_id` and `device_code`.

Author: plotnick

common/src/api/external/mod.rs

@@ -518,6 +518,8 @@ pub enum ResourceType {
     SamlIdentityProvider,
     SshKey,
     ConsoleSession,
+    DeviceAuthRequest,
+    DeviceAccessToken,
     GlobalImage,
     Organization,
     Project,

common/src/sql/dbinit.sql

@@ -1252,37 +1252,36 @@ INSERT INTO omicron.public.user_builtin (
  * OAuth 2.0 Device Authorization Grant (RFC 8628)
  */
 
--- Device authorization requests. In theory these records could
--- (and probably should) be short-lived, and removed as soon as
--- a token is granted.
--- TODO-security: We should not grant a token more than once per record.
+-- Device authorization requests. These records are short-lived,
+-- and removed as soon as a token is granted. This allows us to
+-- use the `user_code` as primary key, despite it not having very
+-- much entropy.
+-- TODO: A background task should remove unused expired records.
 CREATE TABLE omicron.public.device_auth_request (
+    user_code STRING(20) PRIMARY KEY,
     client_id UUID NOT NULL,
     device_code STRING(40) NOT NULL,
-    user_code STRING(63) NOT NULL,
     time_created TIMESTAMPTZ NOT NULL,
-    time_expires TIMESTAMPTZ NOT NULL,
-
-    PRIMARY KEY (client_id, device_code)
+    time_expires TIMESTAMPTZ NOT NULL
 );
 
--- Fast lookup by user_code for verification
-CREATE INDEX ON omicron.public.device_auth_request (user_code);
-
 -- Access tokens granted in response to successful device authorization flows.
 -- TODO-security: expire tokens.
 CREATE TABLE omicron.public.device_access_token (
     token STRING(40) PRIMARY KEY,
     client_id UUID NOT NULL,
     device_code STRING(40) NOT NULL,
     silo_user_id UUID NOT NULL,
-    time_created TIMESTAMPTZ NOT NULL
+    time_requested TIMESTAMPTZ NOT NULL,
+    time_created TIMESTAMPTZ NOT NULL,
+    time_expires TIMESTAMPTZ
 );
 
--- Matches the primary key on device authorization records.
--- The UNIQUE constraint is critical for ensuring that at most
+-- This UNIQUE constraint is critical for ensuring that at most
 -- one token is ever created for a given device authorization flow.
-CREATE UNIQUE INDEX ON omicron.public.device_access_token (client_id, device_code);
+CREATE UNIQUE INDEX ON omicron.public.device_access_token (
+    client_id, device_code
+);
 
 /*
  * Roles built into the system

nexus/src/app/device_auth.rs

@@ -29,8 +29,8 @@
 //!    automatic case, it may supply the `user_code` as a query parameter.
 //! 5. The user logs in using their configured IdP, then enters or verifies
 //!    the `user_code`.
-//! 6. On successful login, the server is notified and responds to the
-//!    poll started in step 3 with a freshly granted access token.
+//! 6. On successful login, the server responds to the poll started
+//!    in step 3 with a freshly granted access token.
 //!
 //! Note that in this flow, there are actually two distinct sets of
 //! connections made to the server: by the client itself, and by the
@@ -45,44 +45,94 @@
 //! but that may change in the future.
 
 use crate::authn::{Actor, Reason};
+use crate::authz;
 use crate::context::OpContext;
+use crate::db::lookup::LookupPath;
 use crate::db::model::{DeviceAccessToken, DeviceAuthRequest};
 use crate::external_api::device_auth::DeviceAccessTokenResponse;
-use omicron_common::api::external::CreateResult;
+
+use omicron_common::api::external::{CreateResult, Error};
+
+use chrono::Utc;
 use uuid::Uuid;
 
 impl super::Nexus {
     /// Start a device authorization grant flow.
     /// Corresponds to steps 1 & 2 in the flow description above.
-    pub async fn device_auth_request(
+    pub async fn device_auth_request_create(
         &self,
         opctx: &OpContext,
         client_id: Uuid,
     ) -> CreateResult<DeviceAuthRequest> {
+        // TODO-correctness: the `user_code` generated for a new request
+        // is used as a primary key, but may potentially collide with an
+        // existing outstanding request. So we should retry some (small)
+        // number of times if inserting the new request fails.
         let auth_request = DeviceAuthRequest::new(client_id);
-        self.db_datastore.device_auth_start(opctx, auth_request).await
+        self.db_datastore.device_auth_request_create(opctx, auth_request).await
     }
 
-    /// Verify a device authorization grant.
+    /// Verify a device authorization grant, and delete the authorization
+    /// request so that at most one token will be granted per request.
+    /// Invoked in response to a request from the browser, not the client.
     /// Corresponds to step 5 in the flow description above.
-    pub async fn device_auth_verify(
+    pub async fn device_auth_request_verify(
         &self,
         opctx: &OpContext,
         user_code: String,
         silo_user_id: Uuid,
     ) -> CreateResult<DeviceAccessToken> {
-        let auth_request =
-            self.db_datastore.device_auth_get_request(opctx, user_code).await?;
-        let client_id = auth_request.client_id;
-        let device_code = auth_request.device_code;
-        let token =
-            DeviceAccessToken::new(client_id, device_code, silo_user_id);
-        self.db_datastore.device_auth_grant(opctx, token).await
+        let (.., authz_request, db_request) =
+            LookupPath::new(opctx, &self.db_datastore)
+                .device_auth_request(&user_code)
+                .fetch()
+                .await?;
+
+        let (.., authz_user) = LookupPath::new(opctx, &self.datastore())
+            .silo_user_id(silo_user_id)
+            .lookup_for(authz::Action::CreateChild)
+            .await?;
+        assert_eq!(authz_user.id(), silo_user_id);
+
+        // Create an access token record.
+        let token = DeviceAccessToken::new(
+            db_request.client_id,
+            db_request.device_code,
+            db_request.time_created,
+            silo_user_id,
+        );
+
+        if db_request.time_expires < Utc::now() {
+            // Store the expired token anyway so that the client
+            // can get a proper "denied" message on its next poll.
+            let token = token.expires(db_request.time_expires);
+            self.db_datastore
+                .device_access_token_create(
+                    opctx,
+                    &authz_request,
+                    &authz_user,
+                    token,
+                )
+                .await?;
+            Err(Error::InvalidRequest {
+                message: "device authorization request expired".to_string(),
+            })
+        } else {
+            // TODO-security: set an expiration time for the valid token.
+            self.db_datastore
+                .device_access_token_create(
+                    opctx,
+                    &authz_request,
+                    &authz_user,
+                    token,
+                )
+                .await
+        }
     }
 
     /// Look up a possibly-not-yet-granted device access token.
     /// Corresponds to steps 3 & 6 in the flow description above.
-    pub async fn device_access_token_lookup(
+    pub async fn device_access_token_fetch(
         &self,
         opctx: &OpContext,
         client_id: Uuid,
@@ -91,7 +141,7 @@ impl super::Nexus {
         use DeviceAccessTokenResponse::*;
         match self
             .db_datastore
-            .device_auth_get_token(opctx, client_id, device_code)
+            .device_access_token_fetch(opctx, client_id, device_code)
             .await
         {
             Ok(token) => Ok(Granted(token)),
@@ -107,12 +157,30 @@ impl super::Nexus {
         opctx: &OpContext,
         token: String,
     ) -> Result<Actor, Reason> {
-        match self.db_datastore.device_access_token_actor(opctx, token).await {
-            Ok(actor) => Ok(actor),
-            // TODO: better error handling
-            Err(_) => Err(Reason::UnknownActor {
-                actor: String::from("from bearer token"),
-            }),
-        }
+        let (.., db_access_token) = LookupPath::new(opctx, &self.db_datastore)
+            .device_access_token(&token)
+            .fetch()
+            .await
+            .map_err(|e| match e {
+                Error::ObjectNotFound { .. } => Reason::UnknownActor {
+                    actor: "from device access token".to_string(),
+                },
+                e => Reason::UnknownError { source: e },
+            })?;
+
+        let silo_user_id = db_access_token.silo_user_id;
+        let (.., db_silo_user) = LookupPath::new(opctx, &self.db_datastore)
+            .silo_user_id(silo_user_id)
+            .fetch()
+            .await
+            .map_err(|e| match e {
+                Error::ObjectNotFound { .. } => {
+                    Reason::UnknownActor { actor: silo_user_id.to_string() }
+                }
+                e => Reason::UnknownError { source: e },
+            })?;
+        let silo_id = db_silo_user.silo_id;
+
+        Ok(Actor::SiloUser { silo_user_id, silo_id })
     }
 }

nexus/src/authz/api_resources.rs

@@ -239,6 +239,8 @@ impl db::model::DatabaseString for FleetRole {
     }
 }
 
+// TODO: refactor synthetic resources below
+
 /// ConsoleSessionList is a synthetic resource used for modeling who has access
 /// to create sessions.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -421,6 +423,60 @@ impl AuthorizedResource for IpPoolList {
     }
 }
 
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub struct DeviceAuthRequestList;
+/// Singleton representing the [`DeviceAuthRequestList`] itself for authz purposes
+pub const DEVICE_AUTH_REQUEST_LIST: DeviceAuthRequestList =
+    DeviceAuthRequestList;
+
+impl oso::PolarClass for DeviceAuthRequestList {
+    fn get_polar_class_builder() -> oso::ClassBuilder<Self> {
+        oso::Class::builder()
+            .with_equality_check()
+            .add_attribute_getter("fleet", |_| FLEET)
+    }
+}
+
+impl AuthorizedResource for DeviceAuthRequestList {
+    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 the DeviceAuthRequestList, only permissions. But we
+        // still need to load the Fleet-related roles to verify that the actor has the
+        // "admin" role on the Fleet.
+        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! {
@@ -602,6 +658,22 @@ authz_resource! {
     polar_snippet = FleetChild,
 }
 
+authz_resource! {
+    name = "DeviceAuthRequest",
+    parent = "Fleet",
+    primary_key = String, // user_code
+    roles_allowed = false,
+    polar_snippet = FleetChild,
+}
+
+authz_resource! {
+    name = "DeviceAccessToken",
+    parent = "Fleet",
+    primary_key = String, // token
+    roles_allowed = false,
+    polar_snippet = FleetChild,
+}
+
 authz_resource! {
     name = "RoleBuiltin",
     parent = "Fleet",

nexus/src/authz/omicron.polar

@@ -355,18 +355,33 @@ resource ConsoleSessionList {
 has_relation(fleet: Fleet, "parent_fleet", collection: ConsoleSessionList)
 	if collection.fleet = fleet;
 
+# Describes the policy for creating and managing device authorization requests.
+resource DeviceAuthRequestList {
+	permissions = [ "create_child" ];
+	relations = { parent_fleet: Fleet };
+	"create_child" if "external-authenticator" on "parent_fleet";
+}
+has_relation(fleet: Fleet, "parent_fleet", collection: DeviceAuthRequestList)
+	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", silo: Silo)
 	if has_role(actor, "external-authenticator", silo.fleet);
 has_permission(actor: AuthenticatedActor, "read", user: SiloUser)
 	if has_role(actor, "external-authenticator", user.silo.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);
 
+has_permission(actor: AuthenticatedActor, "read", device_auth: DeviceAuthRequest)
+	if has_role(actor, "external-authenticator", device_auth.fleet);
+has_permission(actor: AuthenticatedActor, "read", device_token: DeviceAccessToken)
+	if has_role(actor, "external-authenticator", device_token.fleet);
+
 has_permission(actor: AuthenticatedActor, "read", identity_provider: IdentityProvider)
 	if has_role(actor, "external-authenticator", identity_provider.silo.fleet);
 has_permission(actor: AuthenticatedActor, "list_identity_providers", identity_provider: IdentityProvider)

nexus/src/authz/oso_generic.rs

@@ -46,6 +46,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result<Oso, anyhow::Error> {
         IpPoolList::get_polar_class(),
         GlobalImageList::get_polar_class(),
         ConsoleSessionList::get_polar_class(),
+        DeviceAuthRequestList::get_polar_class(),
     ];
     for c in classes {
         info!(log, "registering Oso class"; "class" => &c.name);
@@ -66,6 +67,8 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result<Oso, anyhow::Error> {
         VpcSubnet::init(),
         // Fleet-level resources
         ConsoleSession::init(),
+        DeviceAuthRequest::init(),
+        DeviceAccessToken::init(),
         Rack::init(),
         RoleBuiltin::init(),
         SshKey::init(),