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

Signals for the external acquisition of an api_access_info #2051

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Dimfred
Copy link

@Dimfred Dimfred commented Nov 13, 2019

With this PR I would like to introduce two new signals, to enable the api login process through an external_service.

Signal1: application::login_attempt emitted at the end of application_impl::new_connection.

Signature:     
bool& // should be set by the external service to true, if it is active. 
      // It's bool& to avoid return types 
      // cause this would result in returning a boost::optional
string& // username
string& // password
login_api& // the created login_api (needed to connect the second signal)

Signal2: login_api::api_access_info_external emitted at the beginning of login_api::login.

Signature:
string& // username
fc::optional<api_access_info>& // to be filled by the `external_service`

Logic:

signal::login_attempt is emitted
    => if no service connected: default login
        => signal::api_access_info_external emitted
           => does not set the optional 
           => from here default login
    => else: rerouted login through the external_service
       => service connects to login_api::signal::api_access_info_external           
          => singal::api_access_info_external emitted
             => service trys to find api_access_info to given user
                => if found: login with found
                => else default login

Alternatively this could be done with the first signal alone, but then the service would have to implement the login logic, which I tried to avoid here.

What do you think? Would this be a way to implement that? Open for any discussion.

@pmconrad
Copy link
Contributor

I'm not really clear which problem you are trying to solve here.
Please open an issue describing the problem first, then we'll discuss how to best solve it.

Copy link
Member

@abitmore abitmore left a comment

Choose a reason for hiding this comment

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

At the moment, I do think this is interesting. Unfortunately, the code is incomplete - no test cases or example code to show how to interact with it.

@Dimfred
Copy link
Author

Dimfred commented Aug 8, 2022

Oh this was quite a while ago. If I find time I could look at what I even did there and maybe add tests / an example on how it could be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants