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

Actix frontend #15

Closed
1 task done
HeroicKatora opened this issue Feb 22, 2018 · 26 comments
Closed
1 task done

Actix frontend #15

HeroicKatora opened this issue Feb 22, 2018 · 26 comments
Labels
feature Unimplemented addition help wanted
Projects
Milestone

Comments

@HeroicKatora
Copy link
Owner

HeroicKatora commented Feb 22, 2018

Feature

Actix is arguably a quickly evolving library and its web framework has been proven to be very performant.

The greatest challenge is translation between Future structures and the sequential handling of requests here.

Tracking pull request

  • The development branch has been merged into dev-v0.4.0
@HeroicKatora
Copy link
Owner Author

This will not land in master for now because of the multiple native library dependencies. Rouille and Actix collide with different versions of brotli2 (actix using v0.3.2 while rouille uses v0.2.2). This would best be solved by refactoring this into a workspace instead.

The other problem is ring, used by cookie as well as this library itself. For more informationg and tracking on updates, see the proposed resolution to ring linking issue.

@HeroicKatora
Copy link
Owner Author

It sometimes seems as if a request was blocking and leading to a timeout on all future requests. I have no idea why that is?

@HeroicKatora HeroicKatora added improvement Improve existing implementation help wanted question Best solution is unclear, discussion welcome labels Feb 22, 2018
@HeroicKatora HeroicKatora added this to To do in Frontends Feb 23, 2018
@HeroicKatora HeroicKatora added feature Unimplemented addition and removed improvement Improve existing implementation labels Feb 23, 2018
@HeroicKatora HeroicKatora moved this from To do to In progress in Frontends Mar 22, 2018
@HeroicKatora HeroicKatora added this to the 0.4 milestone Mar 22, 2018
@trsh
Copy link

trsh commented Apr 22, 2018

Want this badly. Need help? How far is this done, and what are common problems?

@HeroicKatora
Copy link
Owner Author

After reworking the frontend to provide an actor based endpoint, everything now works as expected. This means that the basic architecture for this frontend is likely staying as is although the interfaces are not stable yet.

@HeroicKatora HeroicKatora removed the question Best solution is unclear, discussion welcome label May 29, 2018
@joshsharp
Copy link

Hi! I'm building a backend in Actix that'll use OAuth2 and Oxide-auth looks like it'd fit the bill perfectly. I wondered if I could ask, if it's not too pushy to do so, what the status of this frontend is. Is there more left to do, and/or can I get Cargo to use the dev branch so I can take it for a spin? Thanks!

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Jul 24, 2018

You have two choices:

  • Use the 0.4.0-preview.0 version, which includes actix enabled by default (you may want to disable all other frontends).
  • Override the dependency in Cargo.toml and point it to the dev-v0.4.0 branch:
[dependencies]
oxide-auth = { git = 'https://github.com/HeroicKatora/oxide-auth', branch = 'dev-v0.4.0' }

The first option will give you a (hopefully) stable experience while the latter will give you new interfaces sooner. Stable version v0.4.0 should be selected, once published, with both version by using cargo update.

Status on the development is that I'm busy with University end terms until the end of the month and I will continue to work towards the release of 0.4 in August. And if you find anything unpleasant or interface improvements, feel free to open additional issues for those. I will get to them then.

@joshsharp
Copy link

Excellent, thank you! I'll go with the former I think.

@zoidyzoidzoid
Copy link

I tried both of these solutions, and unfortunately couldn't get it working.

I managed to get it working by bumping actix-web and ring in Cargo.toml

@HeroicKatora
Copy link
Owner Author

Thanks for letting me know, I'll update the dependencies for the next version.

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Nov 6, 2018

@zoidbergwill There were some other issues in the examples, see 20b921f if you need to rebase it quickly. Most importantly, in the bottom is an update to the trait that provides .oauth2() since request are now passed as a reference in actix-0.7

@zoidyzoidzoid
Copy link

Awesome, thanks, I'll have a look today. Didn't manage to try it yesterday.

@subhojit777 subhojit777 mentioned this issue Nov 13, 2018
4 tasks
@subhojit777
Copy link

subhojit777 commented Nov 14, 2018

I am using this to implement an oauth server using Actix. I am facing a problem. I have an AppState which is used to initialize the application.

pub struct DbExecutor(pub Pool<ConnectionManager<MysqlConnection>>);

impl Actor for DbExecutor {
    type Context = SyncContext<Self>;
}

pub struct AppState {
    db: Addr<DbExecutor>,
    endpoint: Addr<CodeGrantEndpoint<_>>,
}

I referred this example. Problem is the compiler is not letting me to use _ as type.

I understood that I have to use State as type. But I am not sure whose state, Actixs' or oxide-auth's state is talked about. Need help here.

Sorry if the question is too simple. I am new to Rust, Actix environment. I am trying to understand these stuff.

@HeroicKatora
Copy link
Owner Author

I understood that I have to use State as type. But I am not sure whose state, Actixs' or oxide-auth's state is talked about. Need help here.

The correct type for this would really be the types of the lambda-functions used in the builder of CodeGrantEndpoint, hence you can not name them. Hm, that makes it a bit awkward. I'll come up with a way to do this but am not an expert on Actix either. It should be possible to Box something somewhere, to erase all those lambda types.

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Nov 14, 2018

The text below is only for the case where you need lambdas. If you can use standard methods instead, you can use those as type-parameters in the CodeGrantEndpoint.

I don't think type-erasure is possible on the actix side right now, at least not anywhere on Addr. That means there needs to be a workaround, like so:

use actix::{Actor, Addr, Handler, MailboxError, Message};
use actix::dev::ToEnvelope;
use oxide_auth::frontends::actix::message::*;


trait AbstractAddr<M: Message> {
    fn send(&self, message: M) -> Box<Future<Item=M::Result, Error=MailboxError>>;
}

impl<A, M> AbstractAddr<M> for Addr<A> 
where
    A: Actor + Handler<M>, 
    M: Message + Send + 'static,
    A::Context: ToEnvelope<A, M>,
    M::Result: Send,
{
    fn send(&self, message: M) -> Box<Future<Item=M::Result, Error=MailboxError>> {
        Box::new(self.send(message))
    }
}

trait AbstractEndpoint {
    fn access_token(&self) -> &AbstractAddr<AccessToken>;
    fn authorization_code(&self) -> &AbstractAddr<AuthorizationCode>;
    fn resource_guard(&self) -> &AbstractAddr<Guard>;
}

impl<T> AbstractEndpoint for T 
    where T: AbstractAddr<AccessToken> + AbstractAddr<AuthorizationCode> + AbstractAddr<Guard> 
{
    fn access_token(&self) -> &AbstractAddr<AccessToken> { self }
    fn authorization_code(&self) -> &AbstractAddr<AuthorizationCode> { self }
    fn resource_guard(&self) -> &AbstractAddr<Guard> { self }
}

then you can instead use:

use std::sync::Arc;

pub struct AppState {
    endpoint: Arc<Endpoint + Send + Sync>,
}

let state = AppState {
    endpoint: Arc::new(addr),
};

@HeroicKatora
Copy link
Owner Author

It's good to note this inconvenience. That means I will try to avoid using deduced types in the future for the actix interface. An alternative would be to include the above in the crate itself but it is not as general as I'd like.

@s-panferov
Copy link

@HeroicKatora what's the status of this? Do you know something that prevents this to be merged?

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Nov 26, 2018

The current implementation (v0.4.0-preview.0) works, but doesn't deal well with futures. That was to be expected as the library was written with sync in mind. Currently, I'm trying to refactor everything into a better interface whose completion should mark the true 0.4 release. There are lots of changes in the internals though while trying to keep the frontend interface as stable as possible.

One huge change that should definitely blocks 0.4 is the ability to spawn the issuer and authorizer primitives as actors. Currently, they need to be contained in the actor state of the endpoint.

So, maybe as a Christmas present. Migration guide etc. included.

@subhojit777
Copy link

I decided to use standard method as types. The code looks like:

pub struct AppState<A, B, C>
where
    A: Fn(ClientMap, Storage<RandomGenerator>, TokenSigner, &'static[Scope]) -> AuthorizationFlow,
    B: Fn(ClientMap, Storage<RandomGenerator>, TokenSigner, &'static[Scope]) -> GrantFlow,
    C: Fn(ClientMap, Storage<RandomGenerator>, TokenSigner, &'static[Scope]) -> AccessFlow,
{
    pub db: Addr<DbExecutor>,
    pub endpoint: Addr<CodeGrantEndpoint<A, B, C>>,
}

I have a method that creates a new App like this:

pub fn create_app() -> App<AppState<A, B, C>> {
    // skipped for simplicity
}

The compiler says that it can't find A, B or C in scope. I think I am missing something here due to my less experience in Rust. I think there should be a way to define Fn(ClientMap, Storage<RandomGenerator>, TokenSigner, &'static[Scope]) -> AuthorizationFlow as a type, and then use it, but couldn't find how to do it.

Could you help me here please @HeroicKatora Thanks!

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Dec 14, 2018

In

pub fn create_app() -> App<AppState<A, B, C>> {
    // skipped for simplicity
}

the code needs to refer to specific methods (methods are the only implementors of Fn for which you can name the type, through their own name). So for example, this would work:

fn create_authorization(&ClientMap, &Storage<RandomGenerator>, &TokenSigner, &'static[Scope]) 
    -> AuthorizationFlow 
{
    // …
}

fn create_grant(&ClientMap, &Storage<RandomGenerator>, &TokenSigner, &'static[Scope]) 
    -> GrantFlow 
{
    // …
}

fn create_access(&ClientMap, &Storage<RandomGenerator>, &TokenSigner, &'static[Scope]) 
    -> AcessFlow 
{
    // …
}
pub fn create_app() -> App<AppState<
    fn(&mut State)->AuthorizationFlow, 
    fn(&mut State)->GrantFlow,
    fn(&mut State)->AccessFlow>> 
{
    // skipped for simplicity
}

@subhojit777
Copy link

I am getting errors:

error[E0573]: expected type, found function `create_authorization`
  --> src/lib.rs:62:37
   |
62 | pub fn create_app() -> App<AppState<create_authorization, create_grant, create_access>> {
   |                                     ^^^^^^^^^^^^^^^^^^^^ not a type

error[E0573]: expected type, found function `create_grant`
  --> src/lib.rs:62:59
   |
62 | pub fn create_app() -> App<AppState<create_authorization, create_grant, create_access>> {
   |                                                           ^^^^^^^^^^^^ not a type

error[E0573]: expected type, found function `create_access`
  --> src/lib.rs:62:73
   |
62 | pub fn create_app() -> App<AppState<create_authorization, create_grant, create_access>> {
   |                                                                         ^^^^^^^^^^^^^ not a type

Here's what I did:

pub fn create_authorization(client: &ClientMap, authorizer: &Storage<RandomGenerator>, issuer: &TokenSigner, scopes: &'static[Scope]) -> AuthorizationFlow {
    AuthorizationFlow::new(client, authorizer)
}

pub fn create_grant(client: &ClientMap, authorizer: &Storage<RandomGenerator>, issuer: &TokenSigner, scopes: &'static[Scope]) -> GrantFlow {
    GrantFlow::new(client, authorizer, issuer)
}

pub fn create_access(client: &ClientMap, authorizer: &Storage<RandomGenerator>, issuer: &TokenSigner, scopes: &'static[Scope]) -> AccessFlow {
    AccessFlow::new(issuer, scopes)
}

pub fn create_app() -> App<AppState<create_authorization, create_grant, create_access>> {
}

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Dec 21, 2018

@subhojit777 Sorry. My bad, the type of a raw function is fn(T) -> U, I confused something here. In these specific instances, the type of AppState should be (where State is the state of the CodeGrantEndpoint):

struct AppState {
    pub db: Addr<DbExecutor>,
    pub endpoint: Addr<CodeGrantEndpoint<
        State,
        fn(&mut State) -> AuthorizationFlow,
        fn(&mut State) -> GrantFlow,
        fn(&mut State) -> AccessFlow>>;
}

It's good that these issues come up now, because they shouldn't be in the actual release for v0.4. I hope to find a better way to do setup a endpoint in the next preview version.

Edit: Example usage that compiles and works here: https://github.com/HeroicKatora/oxide-auth/blob/ed716549500df285ad23c68d4987013c92dd0736/examples/authorization_actix.rs

@subhojit777
Copy link

Thanks a lot for the quick help! Did you mean CodeGrantEndoint when you said

(where State is the state of the CodeGrantFrontend)

I wanted to lookup the code for CodeGrantFrontend and couldn't find any reference.

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Dec 24, 2018

Yes, State should be some type you use as state of the CodeGrantEndpoint. I confused some terminology due to reworks, the code is correct :) But github somehow didn't let me edit the comment. (Edit: corrected it now).

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Dec 24, 2018

I'm very happy to report that actix will be getting futures based primitives and endpoints with the next version. The lastest version already contains code for authorization and will be extended with other methods in the near future. Note that the exact interface is not yet stablized but some very handy helper types and methods have shown to be so useful that they were integrated for all other frontends as well. The authorization method also has a complete integration test if you're currently wondering how this will approximately look.

Note that this code portion is not optimized for performance, contains some copy and unecessary boxing but all of that makes the interface much cleaner for now.

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Dec 25, 2018

Alright, just going to complete the example and add some more utilities. Then going for a v0.4-preview.1 release tomorrow. Proper documentation etc is deferred until preview.2 or release.

@HeroicKatora
Copy link
Owner Author

HeroicKatora commented Dec 26, 2018

v0.4.0-preview.1 is out, this will more or less be the interface for actix in v0.4.0. It has basic support for asynchronously handling requests right now as well as starting all primitives (authorizers, registrar, issuers, endpoints) as actors. (And it's technically still Christmas).

@HeroicKatora HeroicKatora mentioned this issue Jan 20, 2019
3 tasks
@HeroicKatora HeroicKatora moved this from In progress to Done in Frontends Jan 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Unimplemented addition help wanted
Projects
No open projects
Frontends
  
Done
Development

No branches or pull requests

6 participants