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

Review Class Structures in TFA Basic and update to D8 concepts #4

Open
doylejd opened this issue Sep 4, 2015 · 4 comments
Open

Review Class Structures in TFA Basic and update to D8 concepts #4

doylejd opened this issue Sep 4, 2015 · 4 comments

Comments

@doylejd
Copy link
Contributor

doylejd commented Sep 4, 2015

There are several concepts in D8 that are not being utilized in the tfa_basic module.

  • Dependency injection. Classes shouldn't need to use \Drupal:: in most cases.
  • TBD
@therealssj
Copy link
Contributor

therealssj commented May 23, 2016

We need to tackle this.
The login architecture is broken right now and when you try to login it gives you a fatal error.
The issue is here in the TFATotp plugin
public function __construct(array $context, array $configuration, $plugin_id, $plugin_definition) {

The plugin is initiated by the ContainerFactory class wherein the definition of plugin construct is
$plugin_class($configuration, $plugin_id, $plugin_definition)

If we remove the $context from TFATotp then how do we access the current user id as that is what the $context contains

@nerdstein
Copy link
Contributor

This can likely be pulled from the main services container, e.g.:

$user = \Drupal::currentUser();

But, please don't lose sight of the dependency injection noted above. That's an important piece here as well to do a proper implementation

@therealssj
Copy link
Contributor

I am not sure how you would accomplish getting the user through any other means
We are limited by the function call $plugin_class($configuration, $plugin_id, $plugin_definition)

@nerdstein
Copy link
Contributor

$context can be removed and replaced with $user = \Drupal::currentUser();

Let's start there to see if this removes the fatal error. We'll work on applying best practices with dependency injection thereafter (for which we can likely bring in the current user).

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

No branches or pull requests

3 participants