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

Password in Redux State. #63

Closed
festelo opened this issue Jul 17, 2018 · 6 comments
Closed

Password in Redux State. #63

festelo opened this issue Jul 17, 2018 · 6 comments

Comments

@festelo
Copy link

festelo commented Jul 17, 2018

I have AuthState with firebaseAuth field. I have a widget called LoginPage, that have state and viewmodel. But when I click Login button, login and password are flushing (TextFormFields becames empty). Should I store password and login in the redux state to restore them in viewmodel? Is it safe?

@immutable
class LoginViewModel {
  final Function(String login, String password) loginCallback;
  final Function loginAnonymouslyCallback;
  final Function openRegisterCallback;
  LoginViewModel({
    this.openRegisterCallback,
    this.loginCallback, 
    this.loginAnonymouslyCallback});
}

class Login extends StatefulWidget {
  Login();
  @override
  _LoginState createState() => new _LoginState();
}

class _LoginState extends State<Login> {
  
  void _signInOrSignUpWithEmail(LoginViewModel viewModel) {
    viewModel.loginCallback(_emailController.text, _passController.text);
  }

  void _signInAnonymously(LoginViewModel viewModel) {
    viewModel.loginAnonymouslyCallback();
  }

  final TextEditingController _emailController = new TextEditingController();
  final TextEditingController _passController = new TextEditingController();
  
  @override
  Widget build(BuildContext context) {
    final theme = Theme.of(context);
    final GlobalKey<FormState> formkey = new GlobalKey<FormState>();
    return StoreConnector<AppState, LoginViewModel>(
      
      converter: (store) => LoginViewModel(
        loginCallback: (login, password) => store.dispatch(new LogInAction(login, password)),
        loginAnonymouslyCallback: () => store.dispatch(new LogInAnonymouslyAction()),
        openRegisterCallback: () => store.dispatch(new OpenRegisterAction()),
      ),

      builder: (context, viewModel) { 
        return Column (
          children: [
            Form(
              key: formkey,
              child: Column(
                children: [
                  TextFormField(
                    controller: _emailController,
                  ),
                  TextFormField(
                    controller: _passController,
                  ),
                ]
              )
            ),
            FlatButton(
              child: Text("Next"),
              onPressed: () { 
                if (formkey.currentState.validate()) 
                  _signInOrSignUpWithEmail(viewModel);
              }
            ),
            FlatButton(
              child: Text(
                "Continue without login",
              ),
              onPressed: () => _signInAnonymously(viewModel),
            )
          ]
        );
      }
    );
  }
}
@festelo
Copy link
Author

festelo commented Jul 17, 2018

Also, how to store the data in the state? I mean, if I want to update the password after every change, I should to write something like this: ``

final fun = () => store.dispatch(new LoginOrPasswordChangedAction(_emailController.text, _passController.text));
_emailController.addListener(fun);
_passController.addListener(fun);

But in this case, after each change, I lose the TextField focus.

@festelo
Copy link
Author

festelo commented Jul 17, 2018

And last question. How to invoke action before another? Should I do it? For example (redux epic):

Stream<dynamic> logoutEpic(Stream<LogOutAction> actions, EpicStore<AppState> store) async* {
  await for (dynamic action in actions) {
    if(action is LogOutAction) {
      yield new PrepareLogOutAction();
//wait while all middlewares handle it.
      yield new FirebaseLogOutAction();
    }
  }
}

I need to remove device id from firestore before user logs out to stop FCM.
Thank you.

@brianegan
Copy link
Owner

Hey there, thanks for writing! Let me see if I can take your Qs one at a time.

I have AuthState with firebaseAuth field. I have a widget called LoginPage, that have state and viewmodel. But when I click Login button, login and password are flushing (TextFormFields becames empty). Should I store password and login in the redux state to restore them in viewmodel? Is it safe?

Overall, I'd personally avoid storing passwords in the State class if you can, for simple security purposes. You might forget to clear this password, or accidentally persist it somewhere, etc.

I'm curious: What is causing the Username & Password fields to be reset when you tap the Login button? I don't see the clear methods being used on your TextEditingControllers anywhere. I'd probably figure out why those are being cleared and prevent that from happening rather than storing the data in the State tree.

Also, how to store the data in the state? I mean, if I want to update the password after every change, I should to write something like this: ``

I generally think the approach of sending 1 LogInAction after the user / pass has been validated is a solid approach. It means you don't need to deal with some of the other complexities of listening to TextEditingControllers or worrying about accidentally leaking password information where it doesn't belong.

And last question. How to invoke action before another? Should I do it? For example (redux epic):

I'd probably just dispatch one action in this case... not quite sure what splitting this one action into 2 actions gives you?

@festelo
Copy link
Author

festelo commented Jul 18, 2018

Thank you for response.

I'm curious: What is causing the Username & Password fields to be reset when you tap the Login button?

When I tap the Login button, the LogInAction is dispatched, then reducer updates the state (the state doesn't contain username or password field) and widget redraws without the ability to restore the username and password.
Oh.. I forgot about Containers, then all is fine.

I generally think the approach of sending 1 LogInAction after the user / pass has been validated is a solid approach.

Ok, thank you.

I'd probably just dispatch one action in this case... not quite sure what splitting this one action into 2 actions gives you?

I want to split middelwares logic, what I mean:
auth_middlewares.dart — contains only middlewares for auth actions (LogInAction, LogOutAction, etc..)
user_middlewares.dart — contains only middlewares for manipulations with user in db (Get/Create/Update/DeleteUserAction and Add/DeleteTokenAction)
So, if I add delete token functions to middleware that catches 'LogOutAction' in auth_middleware, then it go apart from my ideas.

@brianegan
Copy link
Owner

Ah, I gotcha. Yah, in that case, you could definitely dispatch two actions to fulfill that requirement, or consider some Actions as more "Common Actions" that could affect multiple parts of your State tree, such as Logout actions. I don't mind having some "Common Actions" that are handled by different reducers / middleware, since it's generally a bit easier to work with.

That said, if you like the idea of dispatching two actions, no problem at all, you'll just need to do a bit more coordination. In this case, the PrepareLogOutAction would need to contain a Completer. Then, in your user_middleware.dart, after the middleware / epic finishes it's work, it could call action.completer.complete(). Something like this:

// Assumes you're using a TypedEpic for PrepareLogoutActions
Stream<dynamic> logoutEpic(Stream<PrepareLogoutAction> actions, EpicStore<AppState> store) async* {
  await for (PrepareLogoutAction action in actions) {
    await database.deleteUser(action.user);
    action.completer.complete();
  }
}

Then, inside your Epic function that coordinates all of this, you could do something like this:

// This assumes you're using a TypedEpic, which will narrow the actions down to only LogOutAction
Stream<dynamic> logoutEpic(Stream<LogOutAction> actions, EpicStore<AppState> store) async* {
  await for (LogoutAction action in actions) {
    final initialAction = PrepareLogoutAction();
    yield initialAction;
    
    // Await for the initialAction to be completed by the middleware
    await initialAction.completer.future;

    // Then dispatch the FirebaseLogoutAction
    yield new FirebaseLogOutAction();
    }
}

That said, I think this would all be a bit less entangled if you just assume some actions are "Common Actions," and each middleware can handle the action however it wants, without depending on the ordering of the actions. If you need to wait until the first cleanup action finishes before doing the second, the completer solution might be an option, or putting all of this logic into 1 epic might be another option. Then you could just do:

Stream<dynamic> cleanupEpic(Stream<LogoutAction> actions, EpicStore<AppState> store) async* {
  await for (LogoutAction action in actions) {
    await database.deleteUser(action.user);
    await firebase.deleteUser(action.user);
  }
}

That might go against your design philosophy, but I think there are pros and cons to each approach. Hope that helps!

@festelo
Copy link
Author

festelo commented Jul 18, 2018

Thank you again for helping! I think that 'Common actions' is a good approach, despite some cons.
Have a good code.

@festelo festelo closed this as completed Jul 18, 2018
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

2 participants