Skip to content

Commit

Permalink
🐗 Rework how login works
Browse files Browse the repository at this point in the history
All endpoints that would require an interactive login will now return
the gRPC status code for `unauthenticated`. This is a signal to the
client that they can try to call `login` if they have an interactive
context (think UI, CLI). `login` will generate a stream of login
commands (currently only supports Browser to signal to the client to
open a browser).

The interactive login will currently do as many oidc logins as needed to
cover all auth scopes.

Whats left:

I started implementing a tower layer in Bendini (`src/auth.rs`) that
checks for `unauthenticated` and in that case tries to perform an
interactive login process. However, it turns out not to be as nice as I
had hoped. After logging in, we would want to retry the initial request
but the request is not cloneable and therefore this seems
impossible. See hyperium/tonic#733. This is
also the source of the current compilation error that I left in there as
reference.

We should probably do something like David Pedersen suggests and handle
it on a "higher level". Not sure how to make that nice though as the
nice thing of what was started here was that it was 100% automatic and
invisible to higher layers and putting it higher up makes it dependent
on generated types and functions, which tower layers are not since they
run after/before protobuf encoding/decoding respectively. Potentially we
could use some sort of `Deref` implementation where the client is
wrapped in a `Retry` type?

Implement the `wait_for_approval` endpoint in avery.
  • Loading branch information
simonrainerson authored and abbec committed Dec 12, 2021
1 parent dbe31c8 commit 445bfed
Show file tree
Hide file tree
Showing 7 changed files with 413 additions and 126 deletions.
103 changes: 103 additions & 0 deletions clients/bendini/src/auth.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
use std::{
fmt::Display,
future::Future,
pin::Pin,
task::{Context, Poll},
};

use firm_types::{
auth::{authentication_client::AuthenticationClient, interactive_login_command},
tonic::{self, transport::Channel},
};
use futures::{TryFutureExt, TryStreamExt};
use http::Request;
use tower::Service;

type BoxedFuture<T> = Pin<Box<dyn Future<Output = T> + Send>>;

#[derive(Debug, Clone)]
pub struct LoginHandler<InnerService> {
inner: InnerService,
auth_client: AuthenticationClient<Channel>,
}

impl<InnerService> LoginHandler<InnerService> {
pub fn new(inner: InnerService, auth_client: AuthenticationClient<Channel>) -> Self {
LoginHandler { inner, auth_client }
}
}

impl<InnerService, B> Service<Request<B>> for LoginHandler<InnerService>
where
InnerService: Service<Request<B>, Response = http::Response<tonic::transport::Body>>
+ Clone
+ Send
+ 'static,
InnerService::Future: Future + Unpin + Send,
InnerService::Error: Display,
B: Send + 'static,
{
type Response = InnerService::Response;
type Error = tonic::Status;
type Future = BoxedFuture<Result<Self::Response, Self::Error>>;

fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
self.inner
.poll_ready(cx)
.map_err(|e| tonic::Status::unavailable(e.to_string()))
}

fn call(&mut self, req: Request<B>) -> Self::Future {
// This is necessary because tonic internally uses `tower::buffer::Buffer`.
// See https://github.com/tower-rs/tower/issues/547#issuecomment-767629149
// for details on why this is necessary
let clone = self.inner.clone();
let mut inner = std::mem::replace(&mut self.inner, clone);

let mut auth_client = self.auth_client.clone();

Box::pin(async move {
inner
.call(req)
.map_err(|e| tonic::Status::aborted(e.to_string()))
.and_then(|r| async move {
match tonic::Status::from_header_map(r.headers()) {
Some(h) if h.code() == tonic::Code::Unauthenticated => {
auth_client
.login(tonic::Request::new(()))
.and_then(|stream| async move {
// Respond to commands in stream
stream
.into_inner()
.try_for_each(|command| async move {
match command.command {
Some(
interactive_login_command::Command::Browser(b),
) => {
println!("TODO: open browser at url {}", b.url);
Ok(())
}

None => Ok(()), // 🤔
}
})
.and_then(|_| {
// here we would want to retry the earlier
// request but it is not this simple
// unfortunately since the request is not
// Clone: https://github.com/hyperium/tonic/issues/733
inner
.call(req)
.map_err(|e| tonic::Status::aborted(e.to_string()))
})
.await
})
.await
}
_ => Ok(r),
}
})
.await
})
}
}
5 changes: 5 additions & 0 deletions clients/bendini/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#[macro_use]
mod formatting;
mod auth;
mod commands;
mod error;
mod interactive_cert_verifier;
Expand All @@ -19,6 +20,7 @@ use firm_types::{
use futures::TryFutureExt;
use structopt::StructOpt;
use tonic_middleware::HttpStatusInterceptor;
//use tonic_middleware::HttpStatusInterceptor;
use tower::service_fn;

#[cfg(unix)]
Expand Down Expand Up @@ -538,6 +540,9 @@ async fn run() -> Result<(), error::BendiniError> {
Ok(req)
})
}))
// TODO: These should really be opposite order
// That requires some http status interceptor rework
.layer_fn(|s| auth::LoginHandler::new(s, auth_client.clone()))
.layer_fn(HttpStatusInterceptor::new)
.service(channel);

Expand Down
1 change: 1 addition & 0 deletions libraries/rust/tonic-middleware/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ impl HttpStatusInterceptor {
}
}

#[derive(Debug)]
pub struct ResponseFuture {
inner: tonic::transport::channel::ResponseFuture,
}
Expand Down
40 changes: 40 additions & 0 deletions protocols/firm_protocols/auth/auth.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,53 @@ syntax = "proto3";
package firm_protocols.auth;
import "google/protobuf/empty.proto";

/**
* Management service for remote access and external credentials
*/
service Authentication {
/**
* Acquire a new access token for a given scope
*/
rpc AcquireToken (AcquireTokenParameters) returns (Token);
/**
* Authenticate a remote request and optionally create a remote access request
*/
rpc Authenticate (AuthenticationParameters) returns (AuthenticationResponse);
/**
* List non-expired access requests
*/
rpc ListRemoteAccessRequests (RemoteAccessListParameters) returns (RemoteAccessRequests);
/**
* Approve a remote access request
*/
rpc ApproveRemoteAccessRequest (RemoteAccessApproval) returns (RemoteAccessRequest);
/**
* Get a single remote access request
*/
rpc GetRemoteAccessRequest (RemoteAccessRequestId) returns (RemoteAccessRequest);
/**
* Wait for a remote access request to be approved
*/
rpc WaitForRemoteAccessRequest (RemoteAccessRequestId) returns (RemoteAccessRequest);
/**
* Get the identity of the current user
*/
rpc GetIdentity (google.protobuf.Empty) returns (Identity);
/**
* Perform an interactive login. This can be used to cache tokens
* for all token providers that require interaction when logging in.
*/
rpc Login(google.protobuf.Empty) returns (stream InteractiveLoginCommand);
}

message InteractiveLoginCommand {
oneof command {
BrowserAuth browser = 1;
}
}

message BrowserAuth {
string url = 1;
}

message AcquireTokenParameters {
Expand Down
Loading

0 comments on commit 445bfed

Please sign in to comment.