MasoniteFramework / masonite4

Temporary Repository for a Masonite Rewrite for Masonite 4
14 stars 3 forks source link

Fix failing tests #132

Closed girardinsamuel closed 3 years ago

girardinsamuel commented 3 years ago

Tests are failing right now since response refactoring. In Webguard.py there is a mismatch between setting cookies on request and on response...

attempt, attempt_by_id and logout method set and remove cookies both on request and response. I guess it should be always on request ?

@josephmancuso what do you think ? I am not sure here !

girardinsamuel commented 3 years ago
def attempt(self, username, password):
        attempt = self.options.get("model")().attempt(username, password)
        if attempt and not self.options.get("once"):
            self.application.make("response").cookie("token", attempt.remember_token) # --> response ?
            self.application.make("request").set_user(attempt)
            return attempt

def user(self):
    """Get the currently logged in user.

    Returns:
        object|bool -- Returns the current authenticated user object or False or None if there is none.
    """
    token = self.application.make("request").cookie("token")  # --> request ?
    if token and self.model:
        return self.model.where("remember_token", token).first() or False

def attempt_by_id(self, user_id):
    """Login a user by the user ID.

    Arguments:
        user_id {string|int} -- The ID of the user model record.

    Returns:
        object|False -- Returns the current authenticated user object or False or None if there is none.
    """
    attempt = self.options.get("model")().attempt_by_id(user_id)

    if attempt and not self.options.get("once"):
        self.application.make("request").cookie("token", attempt.remember_token)   # --> request ?
        self.application.make("request").set_user(attempt)
        return attempt

    return False

def logout(self):
    """Logout the current authenticated user.

    Returns:
        self
    """
    self.application.make("request").remove_user() 
    return self.application.make("request").delete_cookie("token")   # --> request ?
josephmancuso commented 3 years ago

I'm not sure it needs to check both. I think the reason it does both is because when entering the application, it's set on the request and then on the response, it needs to unset the cookie.

BUT I'm not sure if that's still the case after I refactored how cookies are returned a while back

girardinsamuel commented 3 years ago

I have opened a PR to fix this ! #133