volatiletech / authboss

The boss of http auth.
MIT License
3.85k stars 207 forks source link

Remember me, theft prevention doubt #227

Closed frederikhors closed 5 years ago

frederikhors commented 5 years ago

Issue opened for the creation of a wiki page that summarizes the doubts and problems for newbies (https://github.com/volatiletech/authboss/issues/210).

Reading this answer (https://stackoverflow.com/questions/244882/what-is-the-best-way-to-implement-remember-me-for-a-website/244907#244907) I think something like this can be good for authboss:

If the series is present but the token does not match, a theft is assumed. The user receives a strongly worded warning and all of the user's remembered sessions are deleted.

func (m DBStorer) DelRememberTokens(ctx context.Context, pid string) error {
    authToken := new(AuthToken)
    if _, err := DB.Model(authToken).Where("pid = ?", pid).Delete(); err != nil {
        return err
    }
    return nil
}

func (m DBStorer) UseRememberToken(ctx context.Context, pid, token string) error {
    authToken := new(AuthToken)
        // -------> JUST SEARCH FOR PID HERE
    if err := DB.Model(authToken).Where("pid = ?", pid).Limit(1); err != nil {
        log.Println(err)
        return authboss.ErrTokenNotFound
    }
    // theft prevention (https://stackoverflow.com/a/244907/10088259)
        // -------> SEARCH FOR PID AND TOKEN HERE
    if err := DB.Model(authToken).Where("pid = ?", pid).Where("token = ?", token).Select(); err != nil {
                // ----------------> IF NO TOKEN FOR THIS PID DELETE ALL!
        _ = m.DelRememberTokens(ctx, pid)
        log.Println(err)
        return authboss.ErrTokenNotFound
    }
    if _, err := DB.Model(authToken).Where("pid = ?", pid).Where("token = ?", token).Delete(); err != nil {
        log.Println(err)
        return err
    }
    return nil
}

Am I wrong?

Is there a case where it can be possible?

frederikhors commented 5 years ago

Only now I thought that maybe this issue should be opened on authboss-sample repo.

aarondl commented 5 years ago

This is definitely a fine implementation. Feel free to add the suggestion to the wiki backlog.